Vadim Yanitskiy has posted comments on this change. ( 
https://gerrit.osmocom.org/12410 )

Change subject: GSUP: add end marker to enum osmo_gsup_iei
......................................................................


Patch Set 2:

(2 comments)

https://gerrit.osmocom.org/#/c/12410/2/include/osmocom/gsm/gsup.h
File include/osmocom/gsm/gsup.h:

https://gerrit.osmocom.org/#/c/12410/2/include/osmocom/gsm/gsup.h@102
PS2, Line 102: OSMO_GSUP_IEI_END_MARKER
We usually prefix the end marker by '_' ;)

And BTW, we have some gaps in IEI values (e.g. 0x35 ... 0x40).
Any warranties that this item would be 0x47, and not e.g. 0x36?

Of course, 'gsup' test would fail, but we need to be sure.


https://gerrit.osmocom.org/#/c/12410/2/tests/gsup/gsup_test.c
File tests/gsup/gsup_test.c:

https://gerrit.osmocom.org/#/c/12410/2/tests/gsup/gsup_test.c@404
PS2, Line 404: <= OSMO_GSUP_IEI_END_MARKER - 1
Cosmetic: you could avoid this arithmetic:

  OSMO_ASSERT(... < OSMO_GSUP_IEI_END_MARKER);



--
To view, visit https://gerrit.osmocom.org/12410
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2aab7245e209f0ebd2f33a83d4d181dd3339cb17
Gerrit-Change-Number: 12410
Gerrit-PatchSet: 2
Gerrit-Owner: osmith <[email protected]>
Gerrit-Reviewer: Harald Welte <[email protected]>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Neels Hofmeyr <[email protected]>
Gerrit-Reviewer: Vadim Yanitskiy <[email protected]>
Gerrit-Comment-Date: Fri, 21 Dec 2018 11:41:59 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Reply via email to