Trond, apologies, I'm just getting back to this and had a couple
questions:
On Thu, Feb 21, 2013 at 06:35:46PM +0000, Myklebust, Trond wrote:
> On Thu, 2013-02-21 at 11:38 -0500, J. Bruce Fields wrote:
> > +static int gssx_dec_buffer(struct xdr_stream *xdr,
> > + gssx_buffer *buf)
> > +{
> > + u32 length;
> > + __be32 *p;
> > +
> > + p = xdr_inline_decode(xdr, 4);
> > + if (unlikely(p == NULL))
> > + return -ENOSPC;
> > +
> > + length = be32_to_cpup(p);
> > + p = xdr_inline_decode(xdr, length);
>
> combine for efficiency with the 4 byte allocation above.
This was just a think-o on your part, right? There's no way to decode
the whole thing in one gulp without knowing the length ahead of time.
Or am I missing something?
> > +static int gssx_dec_option_array(struct xdr_stream *xdr,
> > + struct gssx_option_array *oa)
> > +{
> > + struct svc_cred *creds;
> > + u32 count, i;
> > + __be32 *p;
> > + int err;
> > +
> > + p = xdr_inline_decode(xdr, 4);
> > + if (unlikely(p == NULL))
> > + return -ENOSPC;
> > + count = be32_to_cpup(p++);
> > + if (count != 0) {
> > + /* we recognize only 1 currently: CREDS_VALUE */
> > + oa->count = 1;
> > +
> > + oa->data = kmalloc(sizeof(struct gssx_option), GFP_KERNEL);
>
> Sleeping kmallocs inside XDR encode/decode routines is strongly
> discouraged. Particularly so for something that can be preallocated.
And here: I don't mind doing that, but: there's no real problem here as
long as we're only calling this stuff synchronously, right?
--b.
--
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
