[vlc-devel] [PATCH 1/2] httplive: fix Read func. (when caller skips data)

[Hugo Beauzée-Luyssen]

On Wed, Feb 1, 2012 at 1:13 PM, Frederic YHUEL <fyhuel at viotech.net> wrote:
> On Wed, Feb 1, 2012 at 12:28 PM, Hugo Beauzée-Luyssen
> <beauze.h at gmail.com> wrote:
>> On Wed, Feb 1, 2012 at 12:13 PM, Jean-Baptiste Kempf <jb at videolan.org> wrote:
>>> On Tue, Jan 31, 2012 at 01:32:22PM +0100, Frederic YHUEL wrote :
>>>> -            memcpy(p_read + copied, segment->data->p_buffer, len);
>>>> +            if( p_read ) /* otherwise caller skips data */
>>>> +                memcpy(p_read + copied, segment->data->p_buffer, len);
>>>
>>> The code seems wrong if hls_read can be called with p_read at NULL.
>>> Use assert then.
>>>
>>>> -    if (buffer == NULL)
>>>> -    {
>>>> -        /* caller skips data, get big enough buffer */
>>>> -        msg_Warn(s, "buffer is NULL (allocate %d)", i_read);
>>>> -        buffer = calloc(1, i_read);
>>>> -        if (buffer == NULL)
>>>> -            return 0; /* NO MEMORY left*/
>>>> -    }
>>>
>>> What guarrantees us that the buffer will not be NULL ?
>>>
>>> Best regards,
>>>
>>
>> If I'm not mistaking, the all point is to allow buffer to be NULL.
>> This patch looks correct to me, I don't see the point in allocating
>> memory and copying data to it if the data is to be skipped.
>
> +1
>
>> Maybe adding a comment stating that p_read may be NULL at the
>> beginning of the hls_Read function would be better, so one doesn't
>> assume p_read is always != NULL
>>
>
> Indeed, it would probably have helped my point :-)
> Here is a new patch with such a comment.
>
> Thanks,
>

Works for me.

Regards,


-- 
Hugo Beauzée-Luyssen