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 <osm...@sysmocom.de> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Comment-Date: Tue, 26 Feb 2019 13:43:58 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No
