<p><a href="https://gerrit.osmocom.org/c/libosmo-sccp/+/16795">View Change</a></p><p>8 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/libosmo-sccp/+/16795/1//COMMIT_MSG">Commit Message:</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/libosmo-sccp/+/16795/1//COMMIT_MSG@9">Patch Set #1, Line 9:</a> <code style="font-family:monospace,monospace">coe</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">(code)</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/libosmo-sccp/+/16795/1/include/osmocom/sigtran/osmo_ss7.h">File include/osmocom/sigtran/osmo_ss7.h:</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/libosmo-sccp/+/16795/1/include/osmocom/sigtran/osmo_ss7.h@432">Patch Set #1, Line 432:</a> <code style="font-family:monospace,monospace">local_hosts</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">(How about "hosts" instead of "local_hosts" and "host_cnt" instead of "local_host_cnt" (as in osmo_ss7.c)? (same in line below))</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmo-sccp/+/16795/1/include/osmocom/sigtran/osmo_ss7.h@432">Patch Set #1, Line 432:</a> <code style="font-family:monospace,monospace">int osmo_ss7_asp_peer_set_hosts(struct osmo_ss7_asp_peer *peer, void *talloc_ctx, const char* const* local_hosts, size_t local_host_cnt);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">(line is > 100 characters)</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/libosmo-sccp/+/16795/1/src/osmo_ss7.c">File src/osmo_ss7.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/libosmo-sccp/+/16795/1/src/osmo_ss7.c@1116">Patch Set #1, Line 1116:</a> <code style="font-family:monospace,monospace">int osmo_ss7_asp_peer_set_hosts(struct osmo_ss7_asp_peer *peer, void *talloc_ctx, const char* const* hosts, size_t host_cnt)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">(line > 100 characters)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmo-sccp/+/16795/1/src/osmo_ss7.c@1135">Patch Set #1, Line 1135:</a> <code style="font-family:monospace,monospace"></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">(empty line should not be there)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmo-sccp/+/16795/1/src/osmo_ss7.c@1141">Patch Set #1, Line 1141:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> /* Makes no sense to have INET_ANY and specific addresses in the set */<br> for (i = 0; i < peer->host_cnt; i++) {<br> iter_is_any = !peer->host[i] ||<br> !strcmp(peer->host[i], "0.0.0.0");<br> if (new_is_any && iter_is_any)<br> return -EINVAL;<br> if (!new_is_any && iter_is_any)<br> return -EINVAL;<br> }<br> /* Makes no sense to have INET_ANY many times */<br> if (new_is_any && peer->host_cnt)<br> return -EINVAL;<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">(The logic here seems flawed / can be simplified (e.g. move the if (new_is_any ... check up, and we may not even need the for loop if there can only be one INET_ANY address in peer?)... but as I'm writing this, I realize that you just moved the code. So it's fine to leave it as-is in this patch.)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmo-sccp/+/16795/1/src/osmo_ss7.c@2036">Patch Set #1, Line 2036:</a> <code style="font-family:monospace,monospace">rc</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This changes the logic; in the previous version, "osmo_stream_srv_link_set_addrs" would not get executed if the ARRAY_SIZE check failed. Is this on purpose, and if so, why not in a separate commit?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmo-sccp/+/16795/1/src/osmo_ss7.c@2046">Patch Set #1, Line 2046:</a> <code style="font-family:monospace,monospace">rc</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">same here</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmo-sccp/+/16795">change 16795</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/libosmo-sccp/+/16795"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: libosmo-sccp </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I4af2a6915ac57c7baa67343bd9414c65154d67f7 </div>
<div style="display:none"> Gerrit-Change-Number: 16795 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Mon, 13 Jan 2020 10:38:32 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Gerrit-MessageType: comment </div>