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 <jdelv...@suse.de> > > > Cc: Anton Khirnov <an...@khirnov.net> > > > --- > > > 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 ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel