<blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">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.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">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:</p><p style="white-space: pre-wrap; word-wrap: break-word;">https://gerrit.osmocom.org/#/c/osmo-hlr/+/13006/ ("hlr.c: forward GSUP messages between clients")</p><p style="white-space: pre-wrap; word-wrap: break-word;">Then I'll abandon this one.</p><p><a href="https://gerrit.osmocom.org/13048">View Change</a></p><p>3 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/13048/2/src/gsup_router.c">File src/gsup_router.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13048/2/src/gsup_router.c@43">Patch Set #2, Line 43:</a> <code style="font-family:monospace,monospace">talloc_total_size</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">You could alternatively extend struct 'gsup_route' with 'addr_len', and modify gsup_route_add() to s […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Ack</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13048/2/src/gsup_router.c@45">Patch Set #2, Line 45:</a> <code style="font-family:monospace,monospace">a lot of code assumes that addr is also nul-terminated</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Wait, your previous comment also states that "gr->addr is a nul-terminated string" o_O.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Yes, that is redundant.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13048/2/src/gsup_router.c@49">Patch Set #2, Line 49:</a> <code style="font-family:monospace,monospace">Compare addr as non-nul-terminated blob</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Wouldn't this cause a collision when e.g. both (read in hex): […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">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:</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> - 0xff, 0x00, 0x00, 0xbb, 0xaa, 0xff, 0x00  (gr->addr)<br> - 0xff, 0x00, 0x00, 0xbb, 0xaa, 0x77        (addr)</pre><p style="white-space: pre-wrap; word-wrap: break-word;">addrlen would be 6, the check would return false.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/13048">change 13048</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/13048"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-hlr </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I01a45900e14d41bcd338f50ad85d9fabf2c61405 </div>
<div style="display:none"> Gerrit-Change-Number: 13048 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: Vadim Yanitskiy <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 02 Apr 2019 12:55:18 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: No </div>