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

Michael Niedermayer michael at niedermayer.cc
Sat Oct 24 21:05:54 CEST 2015


On Sat, Oct 24, 2015 at 09:02:29PM +0200, Hendrik Leppkes wrote:
> 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.

i think the current code is not correct in a strict reading of the
spec, Ganesh is correct about that
IIUC the spec says the implementation can use any integer type it likes
but it also says elsewhere that things must fit in an int

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No great genius has ever existed without some touch of madness. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151024/39d2995c/attachment.sig>


More information about the ffmpeg-devel mailing list