<p>Patch set 2:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4;">Code-Review -1</span></p><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/19144">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/libosmocore/+/19144/2/src/socket.c">File src/socket.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/libosmocore/+/19144/2/src/socket.c@1526">Patch Set #2, Line 1526:</a> <code style="font-family:monospace,monospace">int osmo_sockaddr_valid(struct osmo_sockaddr *addr)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">the return type/values look strange. Looks like a boolean checker but then it returns 0 on true...</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmocore/+/19144/2/src/socket.c@1532">Patch Set #2, Line 1532:</a> <code style="font-family:monospace,monospace">     if (addr->u.sas.ss_family == AF_INET) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">So indeed you are using one of the fields in a substruct of the union to find out the type. Did you really take care to look this makes sense and that it cannot contain other content based on the union being populated by different ways?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmocore/+/19144/2/src/socket.c@1543">Patch Set #2, Line 1543:</a> <code style="font-family:monospace,monospace">  return -ENOTSUP;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">So in IPv6 it never returns 0, so no ipv6 adr is ever valid.</p><p style="white-space: pre-wrap; word-wrap: break-word;">In any case, this function makes no sense to me. An IP addr being 0.0.0.0 or port 0 is completely fine when binding.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmocore/+/19144">change 19144</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/c/libosmocore/+/19144"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ib6fb050e1bfe3f3a8d8bbe5e762351ce6b7cc48c </div>
<div style="display:none"> Gerrit-Change-Number: 19144 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: lynxis lazus <lynxis@fe80.eu> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Mon, 06 Jul 2020 09:04:29 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Gerrit-MessageType: comment </div>