[asterisk-bugs] [JIRA] (ASTERISK-28809) [patch] res_rtp_asterisk: Avoid absolute value on unsigned subtraction.

[Friendly Automation (JIRA)]

    [ https://issues.asterisk.org/jira/browse/ASTERISK-28809?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=250185#comment-250185 ] 

Friendly Automation commented on ASTERISK-28809:
------------------------------------------------

Change 14088 merged by Friendly Automation:
res_rtp_asterisk: Avoid absolute value on unsigned subtraction.

[https://gerrit.asterisk.org/c/asterisk/+/14088|https://gerrit.asterisk.org/c/asterisk/+/14088]

> [patch] res_rtp_asterisk: Avoid absolute value on unsigned subtraction.
> -----------------------------------------------------------------------
>
>                 Key: ASTERISK-28809
>                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-28809
>             Project: Asterisk
>          Issue Type: Bug
>      Security Level: None
>          Components: Resources/res_rtp_asterisk
>    Affects Versions: 16.9.0, 17.3.0
>         Environment: clang
>            Reporter: Alexander Traud
>            Assignee: Alexander Traud
>              Labels: patch
>         Attachments: clang-6.patch
>
>
> Two years ago, I made a [similar mistake|https://issues.asterisk.org/jira/browse/ASTERISK-27549]. However, this code here was [added|https://github.com/asterisk/asterisk/commit/5bacde37a28c0aac2fb7d6fbbb799331c58f55e7] more than 23 months ago. More than 17 months ago, Asterisk 16 LTS was released. More than 3 months ago, Certified-Asterisk 16 was released. And nobody used the compiler Clang since then? I have an excuse for myself, because I am still with Asterisk 13 LTS. What about other users? In that time, the code was [touched|https://github.com/asterisk/asterisk/commit/87fda066eac730df655bfbf2527d6cd449cd832d] by two core team developers and passed two code reviews. What is wrong with Asterisk?
> {code}res_rtp_asterisk.c:7423:14: warning: taking the absolute value of unsigned type 'unsigned int' has no effect [-Wabsolute-value]
>         } else if ((abs(seqno - rtp->expectedrxseqno) > 100) ||
>                     ^
> res_rtp_asterisk.c:7423:14: note: remove the call to 'abs' since unsigned values cannot be negative
>         } else if ((abs(seqno - rtp->expectedrxseqno) > 100) ||
>                     ^~~{code}
> In this case, the actual problem is that the usual compiler GCC is not asked to warn about [-Wconversion|https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wconversion] or at least [-Wsign-conversion|https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wsign-conversion]. When I enable the latter, I get thousands of warnings. Go figure!
> The attached patch simply silences that warning, because I do not understand what that code actually does. If it wants to detect a distance between two sequence numbers, then the code might be wrong, because sequence numbers wrap at 0xffff; comparisons 100 packets around zero might be wrong.



--
This message was sent by Atlassian JIRA
(v6.2#6252)