[FFmpeg-devel] [PATCH 5/5] all: fix enum definition for large values

Hendrik Leppkes h.leppkes at gmail.com
Sat Oct 24 21:02:29 CEST 2015


On Sat, Oct 24, 2015 at 8:58 PM, Michael Niedermayer
<michael at niedermayer.cc> wrote:
> On Sat, Oct 24, 2015 at 02:41:59PM -0400, Ganesh Ajjanagadde wrote:
>> On Sat, Oct 24, 2015 at 2:33 PM, Michael Niedermayer
>> <michael at niedermayer.cc> wrote:
>> > On Sat, Oct 24, 2015 at 09:29:23AM -0400, Ganesh Ajjanagadde wrote:
>> >> ISO C restricts enumerator values to the range of int. Thus (for instance) 0x80000000
>> >> unfortunately does not work, and throws a warning with -Wpedantic on
>> >> clang 3.7.
>> >>
>> >> This fixes such errors by explicitly casting as an int, doing the
>> >> desired unsigned to signed conversion. This method works on all current
>> >> architectures. Tested with FATE.
>> >>
>> >> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>> >
>> > Simply changing the values to signed is not correct / not sufficient
>> > the code assumes that they are unsigned
>>
>> enums are ints (and hence signed).
>
> this is not true (though thats off topic but you seemed interrested in
> the C spec)
> 6.7.2.2 Enumeration specifiers
> ...
> 4 Each enumerated type shall be compatible with char, a signed integer type, or an
>   unsigned integer type. The choice of type is implementation-defined,110) but shall be
>   capable of representing the values of all the members of the enumeration. The
>   enumerated type is incomplete until after the } that terminates the list of enumerator
>   declarations.
>
> so a enum type can be almost anything the implementation likes it to
> be. 2 enums dont even need to be using the same type and this is not
> just specification talk, gcc does this actually, for example it will
> use (u)int64 as type when things dont fit in an (unsigned) int
> also on at least some embeded platforms gcc will use shorts
> for enums that fit in a short int (no i dont know which exactly but
> i read about people stumbling into this so it must happen on some
> platform)
>
>
>> I doubt code assumed that they are
>> unsigned. If code depended on these being unsigned constants, then
>> there is no way of placing them in an enum. Are you fine with a macro,
>> or do you prefer a static const style? Generally, it seems like FFmpeg
>> prefers the macro method for defining such constants/flags.
>
> if have no preferrance for anything beyond the code working
>

If the current code is correct according to the spec (ie. the enum
gets represented as unsigned or int64), then it should just remain as
it is.

- Hendrik


More information about the ffmpeg-devel mailing list