Change in osmo-hlr[master]: gsup_router.c: gsup_route_find(): support blob

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 Hofmeyr gerrit-no-reply at lists.osmocom.org
Tue Feb 26 13:43:58 UTC 2019


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

Change subject: gsup_router.c: gsup_route_find(): support blob
......................................................................


Patch Set 1:

(3 comments)

I was the one to send you on this journey, and I appreciate this solution.

My conclusion however from what you found is that we currently don't support BLOB addresses properly, especially since you told me that some places store the address as a nul-terminated string pointer without length.

If we want true BLOB support at some point in the future, nipping off the last byte is a new future quirk. Say we fixed all other places, then we might still need to keep this for legacy compat ...

I am not fully decided which way to go, but I lean towards: now always send the terminating nul from GSUP clients as contained in destination addresses (counted in the addr_len), i.e. we contain the nul in the BLOB, but the code should ignore that fact as much as currently possible -- towards a future where no code depends on the nul.

If we end up accepting this patch instead, then the following review applies...

https://gerrit.osmocom.org/#/c/13048/1//COMMIT_MSG
Commit Message:

https://gerrit.osmocom.org/#/c/13048/1//COMMIT_MSG@15
PS1, Line 15: destination name blobs to gsup_route_find().
add "from incoming GSUP messages' destination_name IEs"


https://gerrit.osmocom.org/#/c/13048/1/src/gsup_router.c
File src/gsup_router.c:

https://gerrit.osmocom.org/#/c/13048/1/src/gsup_router.c@42
PS1, Line 42: 	llist_for_each_entry(gr, &gs->routes, list) {
An elaborate comment here would be nice, maybe

   /* gr->addr should be treated as a BLOB, yet for historical reasons may be assumed to be a nul-terminated
      string where the nul is not considered part of the address, especially in incoming GSUP messages.
      Hence, for incoming addr that are not nul-terminated, also compare without the nul. */


https://gerrit.osmocom.org/#/c/13048/1/src/gsup_router.c@50
PS1, Line 50: 		if (gr_addrlen - 1 == addrlen && !memcmp(gr->addr, addr, addrlen))
&& addr[addrlen - 1] != '\0' && gr-addr[gr_addrlen - 1] == '\0'



-- 
To view, visit https://gerrit.osmocom.org/13048
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01a45900e14d41bcd338f50ad85d9fabf2c61405
Gerrit-Change-Number: 13048
Gerrit-PatchSet: 1
Gerrit-Owner: osmith <osmith 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-Comment-Date: Tue, 26 Feb 2019 13:43:58 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190226/d2d39fc2/attachment.htm>


More information about the gerrit-log mailing list