Change in libosmocore[master]: GSUP: add Class IE

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Thu Apr 11 23:24:10 UTC 2019


Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/13576 )

Change subject: GSUP: add Class IE
......................................................................


Patch Set 3:

(2 comments)

I appreciate the review, but in this case...

https://gerrit.osmocom.org/#/c/13576/3/include/osmocom/gsm/gsup.h
File include/osmocom/gsm/gsup.h:

https://gerrit.osmocom.org/#/c/13576/3/include/osmocom/gsm/gsup.h@71
PS3, Line 71: OSMO_GSUP_CLASS_IE
> Looking at this IE, the reader may ask: class of what? […]
It is actually rather absurd, in the face of HUGE GIGANTIC patches waiting for review, we are seriously nitpicking about "Kind" vs "Class" vs "Message Class". This is Bikeshed at its best, if I may say so myself ;)

"Message Class" is saying exactly the same thing but with more words; what part of GSUP is not a Message?

I already once edited all those patches from "KIND" to "CLASS", here, in osmo-hlr and osmo-msc, across entire branch histories; with git format-patch, sed and git am, taking great care to not accidentally modify other sentences containing the word "kind" in unrelated contexts. IMHO that's no sane way to spend my time.


https://gerrit.osmocom.org/#/c/13576/3/include/osmocom/gsm/gsup.h@241
PS3, Line 241: OSMO_GSUP_CLASS_ARRAYSIZE
> Which array? […]
Please see the comment just above it. It explicitly serves as size for an array, hence it is called ARRAYSIZE.

The other may be an End Marker, which it actually isn't: it never is actually used as a marker anywhere. The only place it is used is gsup_test.c for a validity check.

Apologies, but I'd rather not adopt a bad name choice if you don't force me.



-- 
To view, visit https://gerrit.osmocom.org/13576
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: Ic397a9f2c4a7224e47cab944c72e75ca5592efef
Gerrit-Change-Number: 13576
Gerrit-PatchSet: 3
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-Comment-Date: Thu, 11 Apr 2019 23:24:10 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190411/5249583f/attachment.html>


More information about the gerrit-log mailing list