[MPlayer-cvslog] r27195 - trunk/stream/stream_dvd.c

[Mathieu SCHROETER]

Aurelien Jacobs a écrit :
> On Sun, 06 Jul 2008 12:06:56 +0200
> Nico Sabbi <nicola.sabbi at poste.it> wrote:
> 
>> Il giorno ven, 04/07/2008 alle 18.09 +0200, Mathieu SCHROETER ha
>> scritto:
>>> nicodvb a écrit :
>>>> @@ -781,13 +785,18 @@ static int open_s(stream_t *stream,int m
>>>>      /**
>>>>       * Open the disc.
>>>>       */
>>>> -    if(!dvd_device) dvd_device=strdup(DEFAULT_DVD_DEVICE);
>>>> -    dvd_set_speed(dvd_device, dvd_speed);
>>>> +    if(p->device)
>>>> +      dvd_device_current = p->device;
>>>> +    else if(dvd_device)
>>>> +      dvd_device_current = dvd_device;
>>>> +    else
>>>> +      dvd_device_current = strdup(DEFAULT_DVD_DEVICE);
>>>> +    dvd_set_speed(dvd_device_current, dvd_speed);
>>> Hi,
>>>
>>> if dvd_device is NULL and if you use "loadfile dvd://", then
>>> memory is allocated on dvd_device_current.
>>> and after you use "loadfile dvd:////dev/scd1" (for example)
>>> then dvd_device_current will use p->device and previous
>>> allocated memory is _lost_.
>>>
>>> My suggestion was:
>>>
>>>   if(!p->device) {
>>>     if(!dvd_device)
>>>       dvd_device = strdup(DEFAULT_DVD_DEVICE);
>>>     dvd_device_current = dvd_device;
>>>   } else
>>>     dvd_device_current = p->device;
>>>
>>> and malloc is only used on dvd_device (no memleak with
>>> dvd_device_current)..
>>>
>>> Regards,
>>> Mathieu
>>>
>> what about this? simpler and nicer to read IMO
> 
> I haven't read the whole code, so I can't really tell,
> but wouldn't this be enough ?
> 
> -      dvd_device_current = strdup(DEFAULT_DVD_DEVICE);
> +      dvd_device_current = DEFAULT_DVD_DEVICE;
> 
> Aurel

Indeed, your suggestion is the best IMO.

Mathieu