[Asterisk-code-review] sipp, test suite utils: Default media port to an unused port (testsuite[master])

[Benjamin Keith Ford]

Benjamin Keith Ford has posted comments on this change. ( https://gerrit.asterisk.org/8691 )

Change subject: sipp, test_suite_utils: Default media port to an unused port
......................................................................


Patch Set 5: -Code-Review

(2 comments)

https://gerrit.asterisk.org/#/c/8691/5/lib/python/asterisk/utils_socket.py
File lib/python/asterisk/utils_socket.py:

https://gerrit.asterisk.org/#/c/8691/5/lib/python/asterisk/utils_socket.py@233
PS5, Line 233: range(port[0] + 1, port[0] + span +
             :                 step, step)
> If given a port of '4' this code path would not be chosen, but instead the 
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


https://gerrit.asterisk.org/#/c/8691/5/lib/python/asterisk/utils_socket.py@259
PS5, Line 259:         res = self.get_spanned(host, port, socktype, family, span, attempts)
> for the return? No need as the call to reserve can handle an empty list and
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



-- 
To view, visit https://gerrit.asterisk.org/8691
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3da461123afc30e1f5ca12e65d289eaa42d6de00
Gerrit-Change-Number: 8691
Gerrit-PatchSet: 5
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Comment-Date: Thu, 19 Apr 2018 16:00:53 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180419/3e59a5df/attachment-0001.html>