Pau Espin Pedrol has posted comments on this change. ( https://gerrit.osmocom.org/13418 )
Change subject: nanobts: use libosmocore's osmo_store*() for OML attr. patching ...................................................................... Patch Set 1: (2 comments) https://gerrit.osmocom.org/#/c/13418/1/src/osmo-bsc/bts_ipaccess_nanobts_omlattr.c File src/osmo-bsc/bts_ipaccess_nanobts_omlattr.c: https://gerrit.osmocom.org/#/c/13418/1/src/osmo-bsc/bts_ipaccess_nanobts_omlattr.c@202 PS1, Line 202: buf[0] = bts->gprs.nsvc[0].nsvci >> 8; Aren't these 2 lines assuming the code runs on a little endian system? afaiu it converts nsvci to big endian to store it in the message, but that's wrong on BE system. I think we need here something like this: osmo_store16be(&bts->gprs.nsvc[0].nsvci, &buf[0]) Or maybe specs just expect something special? https://gerrit.osmocom.org/#/c/13418/1/src/osmo-bsc/bts_ipaccess_nanobts_omlattr.c@207 PS1, Line 207: osmo_store16be(bts->gprs.nsvc[0].remote_port, &buf[0]); Be careful, you are changing the result/encoding here. Either before there was a bug or you are introducing it now. In any of the cases, please describe and state so in the commit log. Before it used to memcpy, and now you are mangling what you store on little endian systems, so there's a change. -- To view, visit https://gerrit.osmocom.org/13418 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I28cfb09f224072db9889a89923a3da15a6070e2a Gerrit-Change-Number: 13418 Gerrit-PatchSet: 1 Gerrit-Owner: Max <[email protected]> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Max <[email protected]> Gerrit-Reviewer: dexter <[email protected]> Gerrit-CC: Pau Espin Pedrol <[email protected]> Gerrit-Comment-Date: Tue, 26 Mar 2019 16:15:44 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No
