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

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/.

neels gerrit-no-reply at lists.osmocom.org
Thu Oct 1 21:46:29 UTC 2020


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 2:

(1 comment)

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) \
> the point is that the user of a function is the calling function. […]
I would like to log the errors, but we cannot log from here. We don't even know whether the caller initialized logging.

it's not so bad to adjust the error returning if needed. but I thought it would be a drastic improvement compared to how we parse BSSMAP (gsm0808.h). For BSSMAP, every caller does their own tlv parsing and decoding, and logs strings also duplicated by every caller. The argument that we usually return an rc doesn't really count for BSSMAP because we simply don't provide a common decoding API at all, besides the TLV structure definition.

This patch puts the parsing into one common implementation (next to encoding where it belongs) and returns the strings that the caller can then log. No functional difference to how e.g. osmo-bsc parses BSSMAP, except we avoid code dup spread across osmo-bsc and osmo-smlc.

But I get that you see it differently. I could go on arguing that this API does not provide less but more useful information than usually, but let's just move forward.

I can change this to just returing -EINVAL for each parsing error, how we usually do for parsing -- but that would be essentially identical to testing whether the returned char* is non-NULL, just a single-bit success flag.

Or, I think nicest would be to return the IEI where the error occured, which would allow easy logging of at least the rough location of a parsing error. We could also define that the message type is returned in the out-parameter pdu struct to allow relatively easy logging like

  LOGP("error in msg %s IE %s", osmo_bssmap_le_msg_type(pdu->msg_type), osmo_bssmap_le_iei_name(rc)

We could also return integer codes, but make the error message an optional out-arg in osmo_bssap_le_dec(), which would make the error message just an extra feature for bssmap_le.

Another idea would be to add a parsing error struct to the PDU union, providing an rc and a string buffer, which would also allow composing strings with actual values read from the data (in this patch only string constants are possible).



-- 
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: 2
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 21:46:29 +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/ad94073b/attachment.htm>


More information about the gerrit-log mailing list