[FFmpeg-devel] [PATCH] avcodec/fft_template: Fix multiple runtime error: signed integer overflow: -1943918714 - 1935113003 cannot be represented in type 'int'

[Ivan Kalvachev]

On 6/9/17, Michael Niedermayer <michael at niedermayer.cc> wrote:
> On Thu, Jun 08, 2017 at 06:35:07PM -0400, Ronald S. Bultje wrote:
>> Hi,
>>
>> On Thu, Jun 8, 2017 at 6:10 PM, Michael Niedermayer
>> <michael at niedermayer.cc>
>> wrote:
>>
>> > Signed value in
>> > Unsigned
>> > INTeger type
>>
>> [..]
>> > Both SUINT and unsigned should produce identical binaries
>>
>> This seems to go against the rule that code should be as simple as
>> possible.
>>
>> Unsigned is simpler than SUINT if the outcome is the same.
>
> You can simply add the part of my mail here as awnser that you snipped
> away:
>
> "But it makes the code hard to understand and maintain because these
>  values are not positive integers but signed integers. Which for
>  C standard compliance need to be stored in a unsigned type."
>
> A type that avoids the undefinedness of signed but is semantically
> signed is correct, unsigned is not.
>
> If understandable code and maintainable code has no value to you,
> you would favour using single letter variables exclusivly and would
> never use typedef.
> But you do not do that.
>
> I fail to understand why you insist on using unsigned in place of a
> more specific type, it is not the correct nor clean thing to do.


If I understand correctly, the root of the problem is the undefined behavior
of the compiler and the C standard.
Is there any chance that FFmpeg project could initiate a change in the
next C standard, so this thing would be defined?
(and I guess, also define a few other similar things, like signed
right shift, etc...)

It is a matter of fact, that a lot of compiler-defined-things have
been implemented in
certain ways on most compilers and that there are plenty of programs
depend on these
specific implementations and break when a new compiler does things differently
(clang had nice article about it).


About the typedef, is SUINT a standard defined type for workarounding
this specific problem?
I do suspect that some of our fellow developers simply find its name ugly.

Maybe they would be more welcoming if
"suint32_t" like typedef is used?