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

[osmith]

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

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


Patch Set 2:

(3 comments)

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

This sounds like a good solution to me. The patch only seems to make it more complex. I'll update the follow up patch to not depend on this one anymore, and just send the \0 at the end:

https://gerrit.osmocom.org/#/c/osmo-hlr/+/13006/ ("hlr.c: forward GSUP messages between clients")

Then I'll abandon this one.

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

https://gerrit.osmocom.org/#/c/13048/2/src/gsup_router.c@43
PS2, Line 43: talloc_total_size
> You could alternatively extend struct 'gsup_route' with 'addr_len', and modify gsup_route_add() to s […]
Ack


https://gerrit.osmocom.org/#/c/13048/2/src/gsup_router.c@45
PS2, Line 45: a lot of code assumes that addr is also nul-terminated
> Wait, your previous comment also states that "gr->addr is a nul-terminated string" o_O.
Yes, that is redundant.


https://gerrit.osmocom.org/#/c/13048/2/src/gsup_router.c@49
PS2, Line 49: Compare addr as non-nul-terminated blob
> Wouldn't this cause a collision when e.g. both (read in hex): […]
IIRC it would work, because gs->routes would always have \0 at the end ("gr->addr is a nul-terminated string"). So it would look like this:

 - 0xff, 0x00, 0x00, 0xbb, 0xaa, 0xff, 0x00  (gr->addr)
 - 0xff, 0x00, 0x00, 0xbb, 0xaa, 0x77        (addr)

addrlen would be 6, the check would return false.



-- 
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: 2
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-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-CC: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-Comment-Date: Tue, 02 Apr 2019 12:55:18 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190402/a8fca7b6/attachment.htm>