<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: #ffd4d4;">Code-Review -1</span></p><p>(4 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@190">Patch Set #5, Line 190:</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;">            for attempt in range(attempts):<br>                p = get_unused_os_port(host, port, socktype, family)<br><br>                if p != 0 and not self.is_reserved(p, socktype, family):<br>                    res.append(p)<br>                    break<br><br>                if port: return []<br>            else:<br>                raise PortError(socktype, family, attempts)<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I'm not sure this is correct. It looks like it will continuously try to get the same port over and over again, but the "if port: return []" is also nested in here (maybe it's supposed to be outside of the for loop looping over attempts?). If the goal of the function is to find just the ports passed in, I would either remove the attempts loop unless you want it to keep probing the same port, or move the "if port: return []" outside the attempts loop.</p></li><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/8691/5/lib/python/asterisk/utils_socket.py@215">Patch Set #5, Line 215:</a> <code style="font-family:monospace,monospace">number(s)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Looks like this should be just for a single port. Doesn't check to see if it's a list, and gets overwritten later.</p></li><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 style="white-space: pre-wrap; word-wrap: break-word;">This is not returning what I would expect if span is 0. If I use a port of 4 and span 0, I get [5, 4] back.</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 style="white-space: pre-wrap; word-wrap: break-word;">It may be good to check for an empty list here.</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: Wed, 18 Apr 2018 16:46:07 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>