[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-1

(4 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@190
PS5, Line 190:             for attempt in range(attempts):
             :                 p = get_unused_os_port(host, port, socktype, family)
             : 
             :                 if p != 0 and not self.is_reserved(p, socktype, family):
             :                     res.append(p)
             :                     break
             : 
             :                 if port: return []
             :             else:
             :                 raise PortError(socktype, family, attempts)
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.


https://gerrit.asterisk.org/#/c/8691/5/lib/python/asterisk/utils_socket.py@215
PS5, Line 215: number(s)
Looks like this should be just for a single port. Doesn't check to see if it's a list, and gets overwritten later.


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)
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.


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)
It may be good to check for an empty list here.



-- 
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: Wed, 18 Apr 2018 16:46:07 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180418/ba7e4187/attachment.html>