On Tue, Dec 15, 2015 at 08:58:01AM +0100, Jean Delvare wrote: > Hallo Michael, > > On Mon, 14 Dec 2015 23:18:39 +0100, Michael Niedermayer wrote: > > On Mon, Dec 14, 2015 at 07:36:51PM +0100, Jean Delvare wrote: > > > As I understand it, the temporary stack buffer "src_data" was > > > introduced solely to avoid a compiler warning. I believe that a better > > > way to solve this warning it to explicitly cast src->data. This > > > should be somewhat faster, and just as safe. > > > > > > Signed-off-by: Jean Delvare <[email protected]> > > > Cc: Anton Khirnov <[email protected]> > > > --- > > > libavutil/frame.c | 4 +--- > > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > > > --- ffmpeg.orig/libavutil/frame.c 2015-12-14 18:42:06.272234579 +0100 > > > +++ ffmpeg/libavutil/frame.c 2015-12-14 19:05:18.501745387 +0100 > > > @@ -647,7 +647,6 @@ AVFrameSideData *av_frame_get_side_data( > > > > > > static int frame_copy_video(AVFrame *dst, const AVFrame *src) > > > { > > > - const uint8_t *src_data[4]; > > > int i, planes; > > > > > > if (dst->width < src->width || > > > @@ -659,9 +658,8 @@ static int frame_copy_video(AVFrame *dst > > > if (!dst->data[i] || !src->data[i]) > > > return AVERROR(EINVAL); > > > > > > - memcpy(src_data, src->data, sizeof(src_data)); > > > av_image_copy(dst->data, dst->linesize, > > > - src_data, src->linesize, > > > + (const uint8_t **)src->data, src->linesize, > > > > I think this may be a aliasing violation and thus undefined > > not that i like that or consider that sane > > so if someone says iam wrong, i would certainly not be unhappy > > Why would it be an aliasing violation? We do not change the fundamental > type of the pointer, we only add a const where the function wants it. > For more simple pointer types the compiler would do it for us silently.
A. uint8_t and const uint8_t are distinct types
"Any type so far mentioned is an unqualified type. Each unqualified
type has several
qualified versions of its type,38) corresponding to the combinations of
one, two, or all
three of the const, volatile, and restrict qualifiers. The qualified or
unqualified
versions of a type are distinct types that belong to the same type
category and have the
same representation and alignment requirements."
B. a pointer to uint8_t and const uint8_t are not qualified types of the same
type
See the example in 6.2.5
"28 EXAMPLE 1 The type designated as ‘‘float *’’ has type ‘‘pointer to
float’’. Its type category is
pointer, not a floating type. The const-qualified version of this type
is designated as ‘‘float * const’’
whereas the type designated as ‘‘const float *’’ is not a qualified
type — its type is ‘‘pointer to constqualified
float’’ and is a pointer to a qualified type."
C. They are not compatible
"Two types have compatible type if their types are the same. Additional
rules for
determining whether two types are compatible are described in 6.7.2 for
type specifiers,
in 6.7.3 for type qualifiers, and in 6.7.5 for declarators.46)"
D. None of the aliasing exceptions seems to apply
"An object shall have its stored value accessed only by an lvalue
expression that has one of the following types:76)
— atype compatible with the effective type of the object,
— aqualified version of a type compatible with the effective type of
the object,"
(its not a qualified version)
" — a type that is the signed or unsigned type corresponding to the
effective type of the object,
— a type that is the signed or unsigned type corresponding to a
qualified version of the effective type of the object,
— an aggregate or union type that includes one of the aforementioned
types among its members (including, recursively, a member of a subaggregate or
contained union), or
— acharacter type."
(dont apply)
also:
"For two qualified types to be compatible, both shall have the
identically qualified version
of a compatible type; the order of type qualifiers within a list of
specifiers or qualifiers
does not affect the specified type."
"For two pointer types to be compatible, both shall be identically
qualified and both shall
be pointers to compatible types."
I quite possible could misinterpret or miss some parts though ...
>
> Also I can see 12 occurrences of the same cast for this parameter of
> function av_image_copy() in the ffmpeg code already. And over 20 more
> similar casts for similar parameters of other functions
> (ff_combine_frame, swr_convert, copy_picture_field...) So I'm not
> introducing anything new, just proposing one more of the same.
yes, I have no real oppinion on this except that C is insane or I am
and i dont really mind to apply the patch if thats what people prefer.
Any real compiler that follows this litterally and breaks the code is
IMHO a compiler one should avoid (quite independant of it being used
for FFmpeg or other things) unless one wants to language lawyer around
on such things instead of writing usefull code
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
It is what and why we do it that matters, not just one of them.
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list [email protected] http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
