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

[Frederic YHUEL]

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,

-- 
Frédéric
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-HLS-clean-Read-function.patch
Type: text/x-diff
Size: 1885 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120201/0d9f0cc2/attachment.patch>