> 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
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Thanks
Steven
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
