[Asterisk-code-review] res_rtp: Addressing possible rtp range issues (asterisk[16])

[Kevin Harwell]

Attention is currently required from: Michael Bradeen.
Kevin Harwell has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/16637 )

Change subject: res_rtp: Addressing possible rtp range issues
......................................................................


Patch Set 1: Code-Review-1

(1 comment)

File res/res_rtp_asterisk.c:

https://gerrit.asterisk.org/c/asterisk/+/16637/comment/64fcd5cf_17e5cd92 
PS1, Line 3865: 	x = (x == rtpstart && x != (x & ~1)) ? rtpstart + 1 : x;
It takes a little bit of staring to figure out what's going on here, so I think adding a comment explaining things a bit would be good. Or make the code easier to read based on what the checks are doing. For instance might not be a bad idea to just add some IS_EVEN, and/or IS_ODD macros in utils.h or somewhere.

Also I don't think the x == rtpstart check is required? What if 'x' is an odd number not equal to rtpstart?

As well the x != (x & ~1)) check. I think (x & 1) alone will tell you if the value is even or odd so no need for the equality (x != ..) check?

Lastly, and this might be being checked elsewhere, but is it possible for rtpstart to be greater than rtpend, or the variable startplace to be calculated to be? What happens if rtpstart = rptend - 1, and rtpstart is calculated to be an odd number?



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/16637
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I90f07deef0716da4a30206e9f849458b2dbe346b
Gerrit-Change-Number: 16637
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Bradeen <mbradeen at sangoma.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Attention: Michael Bradeen <mbradeen at sangoma.com>
Gerrit-Comment-Date: Thu, 04 Nov 2021 21:50:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20211104/1b7c601b/attachment.html>