Patch Set 1: Code-Review-1 (3 comments)
https://gerrit.osmocom.org/#/c/3187/1/include/osmocom/ranap/iu_client.h File include/osmocom/ranap/iu_client.h: Line 26: /* TODO: It's not needed to store the full SCCP address for each it would make sense to clean this up now, before we will first make this part of a public library while knowing that we will have to break ABI and API again to fix known issues. https://gerrit.osmocom.org/#/c/3187/1/src/iu_client_vty.c File src/iu_client_vty.c: Line 35: "logging asn1-debug (1|0)", this is a view/enable node command, i.e. not configuration. Any "logging" command with view/enable node by definition configures the logging to the current VTY session. Do we have any code that actually ensures that the ASN1 debug log ends up on that VTY (telnet session)? Also, we are changing a global variable, so semantically it behaves completely different to other commands affecting logging to the current vty. If it's not affecting the current VTY session, it should rather be a configuration statement and also saved to the config file. Same is true for "logging asn1-xer-print" below. Even then, existing logging configuration is per log target, and not global. I think this needs some more thought? Line 80: DEFUN(cfg_iu_local_addr_pc, cfg_iu_local_addr_pc_cmd, this should be integrated with the SCCP address book, so that a given Iu user can refer to a named SCCP address book entry to set the local point code. Let's at the very least add a TODO or FIXME comment here. -- To view, visit https://gerrit.osmocom.org/3187 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6a3f7ad15be03fb94689b4af6ccfa828c25f45c0 Gerrit-PatchSet: 1 Gerrit-Project: osmo-iuh Gerrit-Branch: master Gerrit-Owner: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-HasComments: Yes
