Change in libosmocore[master]: add osmo_classmark_* API

[Vadim Yanitskiy]

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

Change subject: add osmo_classmark_* API
......................................................................


Patch Set 2:

(8 comments)

https://gerrit.osmocom.org/#/c/12659/2/src/gsm/gsm48.c
File src/gsm/gsm48.c:

https://gerrit.osmocom.org/#/c/12659/2/src/gsm/gsm48.c@1199
PS2, Line 1199: snprintf(cm1, sizeof(cm1), "no-cm1");
> Again, I suggest to avoid this snprintf() call. Just initialize the cm1[]: […]
PoC: https://pastebin.com/rUvt095q


https://gerrit.osmocom.org/#/c/12659/2/src/gsm/gsm48.c@1230
PS2, Line 1230: const struct osmo_classmark *src, struct osmo_classmark *dst
Let's follow the common (for memcpy, strcpy and memset) sequence here: dst, src.


https://gerrit.osmocom.org/#/c/12659/2/src/gsm/gsm48.c@1234
PS2, Line 1234: dst->classmark1_set
Should we assign false otherwise?


https://gerrit.osmocom.org/#/c/12659/2/src/gsm/gsm48.c@1238
PS2, Line 1238: sizeof(dst->classmark2)
Why not classmark2_len?


https://gerrit.osmocom.org/#/c/12659/2/src/gsm/gsm48.c@1242
PS2, Line 1242: sizeof
same


https://gerrit.osmocom.org/#/c/12659/2/src/gsm/gsm48.c@1269
PS2, Line 1269: } else {
Cosmetic: nesting is not required here, because we return from the main 'if' branch.


https://gerrit.osmocom.org/#/c/12659/2/src/gsm/gsm48.c@1271
PS2, Line 1271: /* Inverted logic for this bit! */
This comment is actually related to the prev. line, please move it.


https://gerrit.osmocom.org/#/c/12659/2/src/gsm/gsm48.c@1303
PS2, Line 1303: return 0;
(just wondering) shouldn't we indicate this 'out of range' case somehow?



-- 
To view, visit https://gerrit.osmocom.org/12659
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: Ifd27bab0380f7ad0c44c719aa6c8bd62cf7b034c
Gerrit-Change-Number: 12659
Gerrit-PatchSet: 2
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-Comment-Date: Mon, 21 Jan 2019 05:28:22 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190121/326c35e6/attachment.htm>