[MPlayer-dev-eng] PATCH: processing "file:///" URL

[Eugene Crosser]

Aurelien Jacobs wrote:
> On Fri, 15 Apr 2005 00:04:58 +0400
> Eugene Crosser <crosser at average.org> wrote:
> 
> 
>>Ismail Donmez wrote:
>>
>>>On Thursday 14 April 2005 19:45, Eugene Crosser wrote:
>>>
>>>
>>>>+       if (strcmp(url->protocol, "file")==0){
>>>>+               char *unescfilename = strdup(url->file);
>>>>+               url_unescape_string(unescfilename,url->file);
>>>>+               return open_stream_full(unescfilename,STREAM_READ,
>>>>+                                              
>>>
>>>options,file_format); >+       }
>>>
>>>
>>>You don't free unescfilename anywhere so this code leaks.
>>
>>That's right.  But it happens only once per run, so this should not be
>>a problem.
> 
> 
> Wrong. It's possible to load as much files as you want at runtime using
> the 'loadfile' slave command.
> 
> 
>>Besides that, as far as I could tell, other existing code
>>that deals with URLs have similar leaks.
> 
> 
> I don't think so, but if that's true, this should be fixed too.

I am not 100% sure, but I have an impression that there are places when 
the function returns without without freeing url.  E.g. in smb code when 
it returns NULL.  OTOH ftp code does free url even if it's going to 
return NULL.

> Moreover, other URLs code at least do
>   url_free(url);
>   url = NULL;
> Your code also need that.
> 
> 
>>I can redo the patch if you people think it is really necessary.
> 
> That would be welcomed.

I changed the code to free both url and unescfilename, please see it it 
is acceptable now.  Diff attached.

Thanks

Eugene
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: mplayer-file-url.diff
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20050415/f416d15a/attachment.txt>