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 <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 01 Sep 2023 12:59:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment