<p>Benjamin Keith Ford <strong>posted comments</strong> on this change.</p><p><a href="https://gerrit.asterisk.org/8691">View Change</a></p><p>Patch set 5:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ddd;">-Code-Review</span></p><p>(2 comments)</p><ul style="list-style: none; padding-left: 20px;"><li><p><a href="https://gerrit.asterisk.org/#/c/8691/5/lib/python/asterisk/utils_socket.py">File lib/python/asterisk/utils_socket.py:</a></p><ul style="list-style: none; padding-left: 20px;"><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/8691/5/lib/python/asterisk/utils_socket.py@233">Patch Set #5, Line 233:</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;">range(port[0] + 1, port[0] + span +<br> step, step)<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">If given a port of '4' this code path would not be chosen, but instead the </blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Wouldn't this path still be taken if span is 0? There aren't any checks for span, just port, and the function description says "If span is zero then just port is checked and/or retrieved", which would still result in the [5, 4] list being checked</p></li><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/8691/5/lib/python/asterisk/utils_socket.py@259">Patch Set #5, Line 259:</a> <code style="font-family:monospace,monospace"> res = self.get_spanned(host, port, socktype, family, span, attempts)</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">for the return? No need as the call to reserve can handle an empty list and</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Just for an early return to not have to call reserve, but this is just a small thing, I'm pretty indifferent on this one</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/8691">change 8691</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.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.asterisk.org/8691"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: testsuite </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I3da461123afc30e1f5ca12e65d289eaa42d6de00 </div>
<div style="display:none"> Gerrit-Change-Number: 8691 </div>
<div style="display:none"> Gerrit-PatchSet: 5 </div>
<div style="display:none"> Gerrit-Owner: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Benjamin Keith Ford <bford@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Corey Farrell <git@cfware.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 19 Apr 2018 16:00:53 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>