This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
laforge gerrit-no-reply at lists.osmocom.orglaforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/20334 )
Change subject: add BSSMAP-LE coding for Location Services
......................................................................
Patch Set 1:
(3 comments)
https://gerrit.osmocom.org/c/libosmocore/+/20334/1/src/gsm/bssmap_le.c
File src/gsm/bssmap_le.c:
https://gerrit.osmocom.org/c/libosmocore/+/20334/1/src/gsm/bssmap_le.c@317
PS1, Line 317: #define PARSE_ERR(ERRMSG) \
> meh, that is not realistic. […]
the point is that the user of a function is the calling function. And a calling function (maybe not today, but tomorrow) may want to take decisions baesd on the type of error. For example, in order to encode the type of error into another protocol (a "cause" value), etc. For sure one doesn't want to !strcmp() the error string in order to find that out.
We do have code that returns "-cause" values in the respective protocol, if it has sufficient cause values. If it doesn't, you either need an enum, or simply log the error directly while returning -1 or -EINVAL or whatever.
This is yet another example where some completely new concept/pattern is introduced, inconsistent with any of the existing code - despite it doing the same as much of our existing code: decoding some protocol. And that concept is implemented all over the Lb code fist, in many occasions, without first giving the team to provide some fedback. Rather, at the end of some weeks a large patch bomb is released and we discover it now. Putting all of us in the sad situation of either accepting this inconsistent approach, or investing additional time to convert it. I really feel sorry for all sides here.
https://gerrit.osmocom.org/c/libosmocore/+/20334/1/src/gsm/bssmap_le.c@335
PS1, Line 335: msgb_put_u8(msg, OSMO_BSSMAP_LE_IEI_APDU);
: l = msgb_put(msg, 2);
: old_tail = msg->tail;
: msgb_put_u8(msg, OSMO_BSSMAP_LE_APDU_PROT_BSSLAP);
: int rc = osmo_bsslap_enc(msg, bsslap);
: if (rc <= 0)
: return -EINVAL;
: osmo_store16be(msg->tail - old_tail, l);
> no, because the IE is appended to existing data
Ack
https://gerrit.osmocom.org/c/libosmocore/+/20334/1/src/gsm/bssmap_le.c@349
PS1, Line 349:
: if (len < 1)
: return "APDU too short";
:
: proto = data[0];
:
: switch (proto) {
: case OSMO_BSSMAP_LE_APDU_PROT_BSSLAP:
: return osmo_bsslap_dec(bsslap, data + 1, len - 1);
: default:
: return "Unimplemented APDU type
> i prefer strings!
see my explanation in other locations. We are not writing a protocol stringifire or something like wireshark where the consumer is a human being. We write protocol decodesr in a library so that other library or application code can react on the results, and hence programmatic, machine-readable results are required. If there is human-readable output, it is optionally in addition, either logged directly or generatd fromt the machine-readable code.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/20334
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I271e59b794bafc0a7ae0eabbf58918f6d7df431d
Gerrit-Change-Number: 20334
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-Comment-Date: Thu, 01 Oct 2020 18:11:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr at sysmocom.de>
Comment-In-Reply-To: laforge <laforge at osmocom.org>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20201001/ed7ff4ab/attachment.htm>