2018-04-19 4:45 GMT+02:00, Steven Liu <l...@chinaffmpeg.org>: > > >> On 19 Apr 2018, at 03:20, wm4 <nfx...@googlemail.com> wrote: >> >> On Wed, 18 Apr 2018 16:10:26 -0300 >> James Almer <jamr...@gmail.com> wrote: >> >>> On 4/18/2018 2:45 PM, Carl Eugen Hoyos wrote: >>>> Hi! >>>> >>>> Attached patch is supposed to fix a warning (and a bug), is this the >>>> right and preferred fix? >>>> >>>> Please comment, Carl Eugen >>>> >>>> >>>> 0001-lavf-dashdec-Do-not-use-memcpy-to-copy-a-struct.patch >>>> >>>> >>>> From cf7d2aefc1a3b3a2e9f578ede43906ed6ee96bfd Mon Sep 17 00:00:00 2001 >>>> From: Carl Eugen Hoyos <ceffm...@gmail.com> >>>> Date: Wed, 18 Apr 2018 19:42:57 +0200 >>>> Subject: [PATCH] lavf/dashdec: Do not use memcpy() to copy a struct. >>>> >>>> Fixes a warning: >>>> libavformat/dashdec.c:1900:65: warning: argument to 'sizeof' in 'memcpy' >>>> call is the same pointer type 'struct fragment *' as the destination; >>>> expected 'struct fragment' or an explicit length >>>> --- >>>> libavformat/dashdec.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c >>>> index 6304ad9..917fb54 100644 >>>> --- a/libavformat/dashdec.c >>>> +++ b/libavformat/dashdec.c >>>> @@ -1897,7 +1897,7 @@ static int init_section_compare_audio(DASHContext >>>> *c) >>>> >>>> static void copy_init_section(struct representation *rep_dest, struct >>>> representation *rep_src) >>>> { >>>> - memcpy(rep_dest->init_section, rep_src->init_section, >>>> sizeof(rep_src->init_section)); >>>> + rep_dest->init_section = rep_src->init_section; >>> >>> This would only copy the pointer. The fact memcpy was used here makes me >>> think the intention was to copy the contents of the struct, so something >>> like >>> >>> *rep_dest->init_section = *rep_src->init_section; >>> >>> or >>> >>> memcpy(rep_dest->init_section, rep_src->init_section, >>> sizeof(*rep_src->init_section)); >>> >>> Would be the correct fix. >> >> The first version would be preferable. But I think the original code >> makes no sense and was never really tested. Looking slightly closer at >> the code, init_section points to a struct that contains a further >> pointer, which would require allocating and dup'ing the memory. >> >> Also the rep_dest->init_sec_buf allocation call isn't even checked. It >> just memcpy's to a NULL pointer. This is some seriously shit code, and >> all of dashdec.c is shit. I'd like to ask Steven Liu (who >> reviewed/pushed the patch that added this copy_init_section code) to >> _actually_ review the patches and to keep up the quality standards in >> this project (which are slightly higher than this). > Yes, that is my mistake, patch welcome and welcome you to contribute code > for refine the dashdec
No (independently of what I think of Vincent's character and tone). You have to either fix the most obvious issues or revert the patch. Carl Eugen _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel