On Tue, 11 Aug 2015 13:32:05 -0400
Anna Schumaker <Anna.Schumaker@xxxxxxxxxx> wrote:
> Hi Jeff,
>
>
> On 08/11/2015 01:13 PM, Jeff Layton wrote:
> > nfs4_label_alloc is a little odd in that a NULL return means "label is
> > not needed" and a PTR_ERR return means an actual error. In at least one
> > place however (nfs_lookup_revalidate) we can end up passing an error
> > value to nfs4_label_free, which will likely lead to an oops.
> >
> > We could fix that one caller, but I think just allowing the free to
> > accept and ignore PTR_ERR values is probably appropriate given how the
> > allocation works.
>
> This makes sense, but I wonder if there are other functions that may-or-may-not allocate a structure without it being considered an error? Does nfs4_label_alloc() match what other functions do?
>
> Thanks,
> Anna
>
AFAICT, label allocation is really a special case, but I didn't do a
survey of the code to look for similar idioms.
> >
> > Signed-off-by: Jeff Layton <jeff.layton@xxxxxxxxxxxxxxx>
> > ---
> > fs/nfs/internal.h | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> > index 9b372b845f6a..44bc298f6216 100644
> > --- a/fs/nfs/internal.h
> > +++ b/fs/nfs/internal.h
> > @@ -312,13 +312,13 @@ nfs4_label_copy(struct nfs4_label *dst, struct nfs4_label *src)
> >
> > return dst;
> > }
> > +
> > static inline void nfs4_label_free(struct nfs4_label *label)
> > {
> > - if (label) {
> > - kfree(label->label);
> > - kfree(label);
> > - }
> > - return;
> > + if (IS_ERR_OR_NULL(label))
> > + return;
> > + kfree(label->label);
> > + kfree(label);
> > }
> >
> > static inline void nfs_zap_label_cache_locked(struct nfs_inode *nfsi)
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Jeff Layton <jlayton@xxxxxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
