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

[Frederic YHUEL]

On Wed, Feb 1, 2012 at 2:52 PM, Hugo Beauzée-Luyssen <beauze.h at gmail.com> wrote:
> 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.
>

Oh I forgot to say that this patch has been tested thoroughly in HLS,
and also in my Smooth Streaming module, which has virtually the same
function, and which implements a forward seek by a call to sms_Read(
s, NULL, len ) (which is not very smart but that's another problem).

If the fact of removing this useless buffer allocation / memcpy were
bad, I should have notice by the time.

Best Regards,

--
Frédéric