Re: [Minios-devel] [UNIKRAFT PATCH] lib/vfscore: stat() - Return EINVAL if pathname is NULL

[Sharan Santhanam <sharan.santhanam@xxxxxxxxx>]

On 8/22/19 10:08 AM, Costin Lupu wrote:
> Hi Sharan,
>
> Thanks for looking into this. Please see inline.
>
> On 8/21/19 7:12 PM, Sharan Santhanam wrote:
> > Hello Costin,
> >
> > Please find the review comment inline.
> >
> > Thanks & Regards
> >
> > Sharan
> >
> > On 8/21/19 3:50 PM, Costin Lupu wrote:
> > > Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
> > > ---
> > >    lib/vfscore/main.c | 2 ++
> > >    1 file changed, 2 insertions(+)
> > >
> > > diff --git a/lib/vfscore/main.c b/lib/vfscore/main.c
> > > index fff233f3..f8692322 100644
> > > --- a/lib/vfscore/main.c
> > > +++ b/lib/vfscore/main.c
> > > @@ -1141,6 +1141,8 @@ LFS64(__xstat);
> > >      int stat(const char *pathname, struct stat *st)
> > >    {
> > > +    if (!pathname)
> > > +        return EINVAL;
> > Shouldn't errno set to EINVAL and return -1.
> Agreed, but since we're on it, would it be better to move this change to
> path_conv() or is it fine here in stat()?
I would keep it at stat. The path_conv is used either from stat or from 
namei_follow_link. The path_conv assumes the path variable to be not 
NULL because in the `namei_follow_link` and its callee there are no 
check for NULL and tracing the path variable further up the path 
variable seem to used even before the call to path_conv. It might be 
necessary to add this error propagation at multiple call site. We could 
add it as we face it.

> > >        return __xstat(1, pathname, st);
> > >    }
> > >    
> > _______________________________________________
> > Minios-devel mailing list
> > Minios-devel@xxxxxxxxxxxxxxxxxxxx
> > https://lists.xenproject.org/mailman/listinfo/minios-devel
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel