Harald Welte has posted comments on this change. ( https://gerrit.osmocom.org/13708 )
Change subject: common/oml.c: refactor Get Attribute Response message generation ...................................................................... Patch Set 1: Code-Review+1 (2 comments) I like the change in principle, but I think variable naming of the old code was more obvious than with the new code. https://gerrit.osmocom.org/#/c/13708/1/src/common/oml.c File src/common/oml.c: https://gerrit.osmocom.org/#/c/13708/1/src/common/oml.c@194 PS1, Line 194: static inline int handle_attrs_trx(struct msgb *msg, const struct gsm_bts_trx *trx, in terms of naming, this could be 'out' or 'msg_out' or 'resp' to indicate clearly that it's the output/response message, and not the input message. Alternatively, put a comment on top of the funciton to make it clear. https://gerrit.osmocom.org/#/c/13708/1/src/common/oml.c@214 PS1, Line 214: /* Push this tag to the list of unreported attributes */ what are "unreported" attributes? I think you can report somethng, but how can you un-report it again? -- To view, visit https://gerrit.osmocom.org/13708 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I349447a43bce360f59e0c6b435906c65167d158b Gerrit-Change-Number: 13708 Gerrit-PatchSet: 1 Gerrit-Owner: Vadim Yanitskiy <[email protected]> Gerrit-Reviewer: Harald Welte <[email protected]> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Comment-Date: Fri, 19 Apr 2019 15:58:39 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: Yes
