Change in libosmocore[master]: add BSSMAP-LE coding for Location Services

[neels]

neels 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) \
> this is highly untypical for our code. […]
meh, that is not realistic. I will not return separate rc depending on the error cause and write another layer of human readable strings around that.

We usually just return -EINVAL and that does not provide any information about which IE had an error.
I know it is (so far) untypical but it is a definite improvement in indicating the cause of decoding errors.

I could imagine returning a negative IEI code to indicate which section had the error as a compromise. still that cannot map all of the error cases reported in these strings.


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);
> you can probably avoid the "let's first reserve two bytes and store a local pointer to whcih we late […]
no, because the IE is appended to existing data


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
> likewise here. Please return error values (cause values, errno, custom error codes, ... […]
i prefer strings!



-- 
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 13:20:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
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/5268bee1/attachment.htm>