laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/18540 )
Change subject: gsm23236: add NRI range utility functions ...................................................................... Patch Set 1: (4 comments) https://gerrit.osmocom.org/c/libosmocore/+/18540/1/include/osmocom/gsm/gsm23236.h File include/osmocom/gsm/gsm23236.h: https://gerrit.osmocom.org/c/libosmocore/+/18540/1/include/osmocom/gsm/gsm23236.h@28 PS1, Line 28: struct llist_head *nri_ranges can probably be 'const' as the nri_ranges list is iterated only for reading? https://gerrit.osmocom.org/c/libosmocore/+/18540/1/include/osmocom/gsm/gsm23236.h@29 PS1, Line 29: struct llist_head *nri_ranges same here, shouldn't it also be const? https://gerrit.osmocom.org/c/libosmocore/+/18540/1/include/osmocom/gsm/gsm23236.h@30 PS1, Line 30: (void *ctx, struct llist_head *nri_range one *could* (not saying you must if you don't like it) create a 'osmo_nri_range_list' or 'osmo_nri_ranges' struct which stores the master llist_head, the nri_bitlen and which also serves as talloc context for any osmo_nri_range allocations. This way the function signatures (and hence usage) would be simplified without having to manually specify ctx and/or nri_bitlen. If the osmo_nri_range had a back-pointer to the ramnge_list/ranges "parent", then even functions only receiving a single osmo_nri_range as argument can get to the nri_Bitlen. https://gerrit.osmocom.org/c/libosmocore/+/18540/1/include/osmocom/gsm/gsm23236.h@42 PS1, Line 42: struct const again? -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/18540 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I68e4156824032772f460042499bbc693380186dc Gerrit-Change-Number: 18540 Gerrit-PatchSet: 1 Gerrit-Owner: neels <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <[email protected]> Gerrit-Comment-Date: Thu, 28 May 2020 09:08:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
