> James,
> 
> No.  You've got it backwards.   0 is false, any non-zero value is true.
> 
> if(desc) would be equal to if (desc != 0).

Ah, you are right. Didn't get much sleep last night :-(

> - Patrick
> 
> On 05/16/2016 01:16 PM, James Simmons wrote:
> > > This looks wrong - You return -EINVAL from sptlrpc_pack_user_desc, but
> > > then
> > > the caller checks "!desc".  Desc will not be null, since you've returned
> > > -EINVAL.
> > Actually 'if (!desc)' is equal to 'if (desc != 0). Yes it can be confusing.
> > I recommend 'if (desc < 0)' instead to make it clearer what is being
> > tested for.
> >
> > > - Patrick
> > >
> > > On 05/16/2016 09:17 AM, Lidza Louina wrote:
> > > > The lustre_msg_buf method could return NULL. Subsequent code didn't
> > > > check if it's null before using it. This patch adds two checks.
> > > >
> > > > Signed-off-by: Lidza Louina <lidza.lou...@oracle.com>
> > > > ---
> > > >    drivers/staging/lustre/lustre/ptlrpc/sec.c       | 2 ++
> > > >    drivers/staging/lustre/lustre/ptlrpc/sec_plain.c | 9 +++++++--
> > > >    2 files changed, 9 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/lustre/lustre/ptlrpc/sec.c
> > > > b/drivers/staging/lustre/lustre/ptlrpc/sec.c
> > > > index 187fd1d..e6fedc3 100644
> > > > --- a/drivers/staging/lustre/lustre/ptlrpc/sec.c
> > > > +++ b/drivers/staging/lustre/lustre/ptlrpc/sec.c
> > > > @@ -2195,6 +2195,8 @@ int sptlrpc_pack_user_desc(struct lustre_msg *msg,
> > > > int
> > > > offset)
> > > >     struct ptlrpc_user_desc *pud;
> > > >    
> > > >         pud = lustre_msg_buf(msg, offset, 0);
> > > > +       if (!pud)
> > > > +               return -EINVAL;
> > > >    
> > > >     pud->pud_uid = from_kuid(&init_user_ns, current_uid());
> > > >     pud->pud_gid = from_kgid(&init_user_ns, current_gid());
> > > > diff --git a/drivers/staging/lustre/lustre/ptlrpc/sec_plain.c
> > > > b/drivers/staging/lustre/lustre/ptlrpc/sec_plain.c
> > > > index 37c9f4c..51741c8 100644
> > > > --- a/drivers/staging/lustre/lustre/ptlrpc/sec_plain.c
> > > > +++ b/drivers/staging/lustre/lustre/ptlrpc/sec_plain.c
> > > > @@ -542,6 +542,7 @@ int plain_alloc_reqbuf(struct ptlrpc_sec *sec,
> > > >    {
> > > >     __u32 buflens[PLAIN_PACK_SEGMENTS] = { 0, };
> > > >     int alloc_len;
> > > > +       int desc;
> > > >    
> > > >     buflens[PLAIN_PACK_HDR_OFF] = sizeof(struct plain_header);
> > > >     buflens[PLAIN_PACK_MSG_OFF] = msgsize;
> > > > @@ -574,8 +575,12 @@ int plain_alloc_reqbuf(struct ptlrpc_sec *sec,
> > > >     lustre_init_msg_v2(req->rq_reqbuf, PLAIN_PACK_SEGMENTS, buflens,
> > > >     NULL);
> > > >     req->rq_reqmsg = lustre_msg_buf(req->rq_reqbuf, PLAIN_PACK_MSG_OFF,
> > > >     0);
> > > >    -    if (req->rq_pack_udesc)
> > > > -               sptlrpc_pack_user_desc(req->rq_reqbuf, 
> > > > PLAIN_PACK_USER_OFF);
> > > > +       if (req->rq_pack_udesc) {
> > > > +               desc = sptlrpc_pack_user_desc(req->rq_reqbuf,
> > > > +                                             PLAIN_PACK_USER_OFF);
> > > > +               if (!desc)
> > > > +                       return desc;
> > > > +       }
> > > >    
> > > >         return 0;
> > > >    }
> > > _______________________________________________
> > > lustre-devel mailing list
> > > lustre-de...@lists.lustre.org
> > > http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org
> > >
> 
> 
> 
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to