Attention is currently required from: pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31304 )
Change subject: Includes: Remove enum/include from libosmocore instead ...................................................................... Patch Set 7: (4 comments) File include/osmocom/bsc/gsm_data.h: https://gerrit.osmocom.org/c/osmo-bsc/+/31304/comment/e264bb73_1a38a20c PS6, Line 657: uint16_t parameter[_NUM_NM_RLC_PAR_OFFSET]; > why changing the name? I don't see the point in adding _OFFSET it just makes > stuff larger with no be […] I added the suffix, because this makes clear (self-documenting code and such) that their values aren't to be touched, since these values are used as offsets. The rest of the name change went along with moving the enums from separate headers (iirc all with basename `gsm_data.h`) in `osmo-bsc`/`osmo-bts` (and possibly `osmo-pcu`) to `libosmocore.git:include/osmocom/gsm/protocol/gsm_12_21.h` to avoid duplicating the same enum values across multiple projects when it makes more sense to have them centrally in a library. File include/osmocom/bsc/gsm_data.h: https://gerrit.osmocom.org/c/osmo-bsc/+/31304/comment/e499b600_ef3bdf5e PS1, Line 654: enum gprs_cs { > IIUC the one under gsm_44_06. […] ah yes they have different values and I grepped for the bitmask; it isn't always accessed by use of the enum identifiers either File src/osmo-bsc/bts_ipaccess_nanobts_omlattr.c: https://gerrit.osmocom.org/c/osmo-bsc/+/31304/comment/6557e52d_ba2cbf90 PS6, Line 173: for (offset = NM_RLC_T3142_OFFSET; offset <= NM_CV_COUNTDOWN_OFFSET; > why not simply memcpy? Tbh. we don't have to have a for loop here, I don't care much either way and it might be better to keep it the way it was: Line by line assignments. I have discarded the loop usage. Makes the patch easier to read as well since it's focussing on the core change. `memcpy()` doesn't make much sense since the source is an array of `uint16_t` and the destination is a regular byte-array. https://gerrit.osmocom.org/c/osmo-bsc/+/31304/comment/41b755e1_d62a592d PS6, Line 174: ++offset) > please don't use preincrement unless it really has a purpose over > postincrement (code convention in […] Alright I'll keep it in mind, thank you -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31304 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I2883aee12b501a717d5acecd5638882724336e72 Gerrit-Change-Number: 31304 Gerrit-PatchSet: 7 Gerrit-Owner: arehbein <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <[email protected]> Gerrit-Attention: pespin <[email protected]> Gerrit-Comment-Date: Fri, 01 Sep 2023 12:59:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein <[email protected]> Comment-In-Reply-To: pespin <[email protected]> Gerrit-MessageType: comment
