Patch Set 1: Code-Review-1 (4 comments)
https://gerrit.osmocom.org/#/c/2786/1/src/common/oml.c File src/common/oml.c: Line 173: static inline struct msgb *add_trx_attr(struct gsm_bts_trx *trx) naming. The function is called add_something. But In reality it allocates a message and returns it? Line 186: static inline struct msgb *add_trx_power(struct gsm_bts_trx *trx) same here about naming. Line 208: static inline int cleanup_attr(struct msgb *a, int length, uint8_t *out) I once again fail to understand what you're doing here, and in absence of comments or any expressive name, I have nothign to help me... Line 236: ba = add_trx_attr(gsm_bts_trx_num(bts, mo->obj_inst.trx_nr)); why are we calling a function which allocates a message to put a single information element into the message, and then later memcpy()ing out its contents and freeing it again? -- To view, visit https://gerrit.osmocom.org/2786 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9f72305bbf1ab74745bffac1bee9f539f5a6de32 Gerrit-PatchSet: 1 Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Owner: Max <msur...@sysmocom.de> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-HasComments: Yes
