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

[Alexander Traud (JIRA)]

Alexander Traud created ASTERISK-28809:
------------------------------------------

             Summary: [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: 17.3.0, 16.9.0
         Environment: clang
            Reporter: Alexander Traud
         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)