Libguestfs - Jan 2020 - Re: [PATCH 3/3] docs: don't perform lookup on absolute paths

On Monday, 27 January 2020 10:39:34 CET Tomáš Golembiovský
wrote:
> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> ---
>  podwrapper.pl.in | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/podwrapper.pl.in b/podwrapper.pl.in
> index f12a173f..1e4aa149 100755
> --- a/podwrapper.pl.in
> +++ b/podwrapper.pl.in
> @@ -689,6 +689,8 @@ sub find_file
>      my $use_path = shift;
>      local $_;
>  
> +    return $input if File::Spec->file_name_is_absolute($input) and -f
$input;
Do you really need to use file_name_is_absolute? -f seems to work fine
also with absolute paths. In case the path is relative, -f will be fine
too, as...
>      my @search_path = (".");
>      push (@search_path, @paths) if $use_path;
>      foreach (@search_path) {
... the first search path is always '.'.

-- 
Pino Toscano

On Mon, Jan 27, 2020 at 12:17:42PM +0100, Pino Toscano
wrote:
> On Monday, 27 January 2020 10:39:34 CET Tomáš Golembiovský wrote:
> > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> > ---
> >  podwrapper.pl.in | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/podwrapper.pl.in b/podwrapper.pl.in
> > index f12a173f..1e4aa149 100755
> > --- a/podwrapper.pl.in
> > +++ b/podwrapper.pl.in
> > @@ -689,6 +689,8 @@ sub find_file
> >      my $use_path = shift;
> >      local $_;
> >  
> > +    return $input if File::Spec->file_name_is_absolute($input) and
-f $input;
> 
> Do you really need to use file_name_is_absolute? -f seems to work fine
> also with absolute paths. In case the path is relative, -f will be fine
> too, as...
It's all about skipping the code below. The '.' will turn your nice
absolute path '/foo/bar' into relative path './foo/bar' and the
lookup
will fail.

Instead of letting it fall through maybe something like this would be
more expressive:

    if File::Spec->file_name_is_absolute($input) {
        return $input if -f $input;
        die "$progname: $input: cannot find input file"
    }
> 
> >      my @search_path = (".");
> >      push (@search_path, @paths) if $use_path;
> >      foreach (@search_path) {
> 
> ... the first search path is always '.'.
> 
> -- 
> Pino Toscano

> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs

-- 
Tomáš Golembiovský <tgolembi@redhat.com>

On Monday, 27 January 2020 12:37:38 CET Tomáš Golembiovský
wrote:
> On Mon, Jan 27, 2020 at 12:17:42PM +0100, Pino Toscano wrote:
> > On Monday, 27 January 2020 10:39:34 CET Tomáš Golembiovský wrote:
> > > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> > > ---
> > >  podwrapper.pl.in | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/podwrapper.pl.in b/podwrapper.pl.in
> > > index f12a173f..1e4aa149 100755
> > > --- a/podwrapper.pl.in
> > > +++ b/podwrapper.pl.in
> > > @@ -689,6 +689,8 @@ sub find_file
> > >      my $use_path = shift;
> > >      local $_;
> > >  
> > > +    return $input if
File::Spec->file_name_is_absolute($input) and -f $input;
> > 
> > Do you really need to use file_name_is_absolute? -f seems to work fine
> > also with absolute paths. In case the path is relative, -f will be
fine
> > too, as...
> 
> It's all about skipping the code below. The '.' will turn your
nice
> absolute path '/foo/bar' into relative path './foo/bar' and
the lookup
> will fail.
Oh sorry, most probably I did not explain properly what I meant.

Since -f works on both absolute and relative paths, my suggestion is
to change your line into a simpler:

  return $input if $input;

and most probably removing '.' from the search loop below.

-- 
Pino Toscano