Patch Set 6: (3 comments)
https://gerrit.osmocom.org/#/c/2545/6/src/common/oml.c File src/common/oml.c: Line 174: uint16_t i, r = 1; /* byte 0 is reserved for unsupported attributes counter */ i think the variables are named un-intuitive. Using 'i' as a loop counter in a for loop, ok. But what is "r" here? Something like the number of output bytes or output attributes? If so, please name it more descriptively. Line 213: return i; why do we return "i" and just set "i" to something computed from "r" above, rather than returning r directly? or the other way: Why are we re-using the 'i' loop counter for something that's not the loop count and then returning it? I think this makes the code hard to understand. Line 220: uint8_t resp[MAX_VERSION_LENGTH * attr_len * 2]; /* heuristic for ARI space requirements */ ARI? -- To view, visit https://gerrit.osmocom.org/2545 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I09f95ed995fab5def9dc6e8cc201012fba4db28d Gerrit-PatchSet: 6 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-Reviewer: Max <msur...@sysmocom.de> Gerrit-HasComments: Yes
