Change in osmo-bsc[master]: nanobts: use libosmocore's osmo_store*() for OML attr. patching

Pau Espin Pedrol gerrit-no-reply at lists.osmocom.org
Tue Mar 26 16:15:44 UTC 2019


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 <msuraev at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-CC: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Comment-Date: Tue, 26 Mar 2019 16:15:44 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190326/df4112ec/attachment.html>


More information about the gerrit-log mailing list