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

[Alexander Strasser]

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.

  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.

Thanks for the review,
  Alexander
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140103/37d55036/attachment.asc>