osmith 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 '_' ;)

Thanks, fixed.

> Any warranties that this item would be 0x47, and not e.g. 0x36?

Yes, the C standard says:
"If the first enumerator has no =, the value of its enumeration constant is 0. 
Each subsequent enumerator with no = defines its enumeration constant as the 
value of the constant expression obtained by adding 1 to the value of the 
previous enumeration constant."

See this draft of C99 for example, in 6.7.2.2 Enumeration specifiers:
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf

(Turns out non-drafts are not published for free.)


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: […]
Right, that's better :)



--
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-Reviewer: osmith <[email protected]>
Gerrit-Comment-Date: Fri, 21 Dec 2018 12:14:38 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Reply via email to