[FFmpeg-devel] [PATCH 1/3] lavf/file: file_check: Handle paths that start with file:

[Stefano Sabatini]

On date Friday 2014-01-03 21:53:02 +0100, Alexander Strasser encoded:
> Hi Stefano,
> 
> On 2014-01-03 18:19 +0100, Stefano Sabatini wrote:
> > On date Friday 2014-01-03 17:35:10 +0100, Stefano Sabatini encoded:
> > > Subject nit: lavf/file: file_check: handle paths that start with "file:"
> > > 
> > > Also you should tell how this is handling it.
> 
>   OK, I will include the double quotes. I had them initially but dropped
> them because I just thought that the language is so explicit concerning
> structure at that point of the sentence that no explicit quotation marks
> are needed.
> 
>   Will also write a detail message that explains how things are handled.
> 
> > > 
> > > On date Thursday 2014-01-02 20:21:23 +0100, Alexander Strasser encoded:
> > > > Fix part of ticket #3249.
> > > > 
> > > > TODO: file_check is also used in pipe protocol,
> > > >       but this patch should not hurt AFAICT
> > > > 
> > > > Signed-off-by: Alexander Strasser <eclipse7 at gmx.net>
> > > > ---
> > > >  libavformat/file.c | 15 ++++++++++-----
> > > >  1 file changed, 10 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/libavformat/file.c b/libavformat/file.c
> > > > index 2defc75..acae281 100644
> > > > --- a/libavformat/file.c
> > > > +++ b/libavformat/file.c
> > > > @@ -104,25 +104,30 @@ static int file_get_handle(URLContext *h)
> > > >  
> > > >  static int file_check(URLContext *h, int mask)
> > > >  {
> > > > -#if HAVE_ACCESS && defined(R_OK)
> > > >      int ret = 0;
> > > > -    if (access(h->filename, F_OK) < 0)
> > > 
> > > > +    const char * filename = h->filename;
> > > 
> > > nit: const char *filename = ...
> > > 
> > > > +    av_strstart(filename, "file:", &filename);
> > > 
> > > So basically this is stripping "file:" from the filename. I'm not sure
> > > it is correct to mangle the provided input file (the prefix should be
> > > already stripped by the framework code). What happens for example if
> > > the file starts with "file:"?
> > 
> > OTOH in that case the user can still do: file:$FILENAME to protect
> > names containing an initial "file:" (e.g. file:I:am:a:file) so I guess
> > this solution is acceptable and is consistent with file_open().
> > 

> > We should probably clarify docs about this.
> 
>   Yes, you already answered your question here. Using a file that starts
> with "file:" will not work unless you use "file:file:" or simply use
> "./file:" or similar ways to specify the file name.
> 
>   It has nothing to do with this patch and I fear it is not documented.

Docu-patch sent.

>   For your other remark about the {}, the #else part still declares a
> variable. This avoids having statements and declarations mixed in a
> block after my patch. I did not add them into the #else part because
> this way it will need no changes if variables are added again in the
> #if part.

I think the patch is fine as is once you fix/extend the commit
message, feel free to push if there are no further comments. Thanks.
-- 
FFmpeg = Fundamentalist and Faithful Minimalistic Picky Exxagerate Genius