On Thu, 2021-01-21 at 12:23 -0500, David Wysochanski wrote:
> On Thu, Jan 21, 2021 at 12:05 PM Trond Myklebust
> <trondmy@xxxxxxxxxxxxxxx> wrote:
> >
> > On Thu, 2021-01-21 at 11:20 -0500, Dave Wysochanski wrote:
> > > Remove duplicated helper functions to parse opaque XDR objects
> > > and place inside the xdr.h file. Also update comment which
> > > is wrong since lockd is not the only user of xdr_netobj.
> > >
> > > Signed-off-by: Dave Wysochanski <dwysocha@xxxxxxxxxx>
> > > ---
> > > include/linux/sunrpc/xdr.h | 33
> > > +++++++++++++++++++++++++++++++--
> > > net/sunrpc/auth_gss/auth_gss.c | 29 -----------------------
> > > ----
> > > --
> > > net/sunrpc/auth_gss/gss_krb5_mech.c | 29 -----------------------
> > > ----
> > > --
> > > 3 files changed, 31 insertions(+), 60 deletions(-)
> > >
> > > diff --git a/include/linux/sunrpc/xdr.h
> > > b/include/linux/sunrpc/xdr.h
> > > index 19b6dea27367..8ef788ff80b9 100644
> > > --- a/include/linux/sunrpc/xdr.h
> > > +++ b/include/linux/sunrpc/xdr.h
> > > @@ -25,8 +25,7 @@
> > > #define XDR_QUADLEN(l) (((l) + 3) >> 2)
> > >
> > > /*
> > > - * Generic opaque `network object.' At the kernel level, this
> > > type
> > > - * is used only by lockd.
> > > + * Generic opaque `network object.'
> > > */
> > > #define XDR_MAX_NETOBJ 1024
> > > struct xdr_netobj {
> > > @@ -34,6 +33,36 @@ struct xdr_netobj {
> > > u8 * data;
> > > };
> > >
> > > +static inline const void *
> > > +simple_get_bytes(const void *p, const void *end, void *res,
> > > size_t
> > > len)
> > > +{
> > > + const void *q = (const void *)((const char *)p + len);
> > > + if (unlikely(q > end || q < p))
> > > + return ERR_PTR(-EFAULT);
> > > + memcpy(res, p, len);
> > > + return q;
> > > +}
> > > +
> > > +static inline const void *
> > > +simple_get_netobj(const void *p, const void *end, struct
> > > xdr_netobj
> > > *dest)
> > > +{
> > > + const void *q;
> > > + unsigned int len;
> > > +
> > > + p = simple_get_bytes(p, end, &len, sizeof(len));
> > > + if (IS_ERR(p))
> > > + return p;
> > > + q = (const void *)((const char *)p + len);
> > > + if (unlikely(q > end || q < p))
> > > + return ERR_PTR(-EFAULT);
> > > + dest->data = kmemdup(p, len, GFP_NOFS);
> > > + if (unlikely(dest->data == NULL))
> > > + return ERR_PTR(-ENOMEM);
> > > + dest->len = len;
> > > + return q;
> > > +}
> > > +
> >
> > Hmm... I'm not too keen on having these in sunrpc/xdr.h. For one
> > thing,
> > they do not describe objects that are XDR encoded. Secondly, their
> > naming isn't really consistent with any of the existing conventions
> > for
> > xdr.
> >
> > Can we please rather just create a new header file that is private
> > in
> > net/sunrpc/auth_gss/ ?
> >
>
> Sure. Do you have any preference for the name?
> internal.h
> auth_gss.h
> something else...
>
It is unfortunate that we already have an auth_gss.h in
include/linux/sunrpc. How about auth_gss_internal.h instead?
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
