On Sat, Mar 22, 2014 at 11:43:36PM -0700, Christoph Hellwig wrote:
> > #define RESERVE_SPACE(nbytes) do { \
> > - p = resp->xdr.p; \
> > - BUG_ON(p + XDR_QUADLEN(nbytes) > resp->xdr.end); \
> > + p = xdr_reserve_space(&resp->xdr, nbytes); \
> > + BUG_ON(!p); \
> > } while (0)
> > -#define ADJUST_ARGS() resp->xdr.p = p
> > +#define ADJUST_ARGS() WARN_ON_ONCE(p != resp->xdr.p) \
>
> I think the code would become more clear if you'd start expanding these
> macros. IF the RESERVE_SPACE BUG_ON is important enough it should move
> into xdr_reserve_space or a variant thereof, or otherweise just removed.
Yeah, those will go later.
Ideas for how to sequence these changes better welcomed.
> > + if (nfserr) {
> > + xdr->p -= 2;
> > + xdr->iov->iov_len -= 8;
>
>
> > + if (nfserr) {
> > + xdr->p--;
> > + xdr->iov->iov_len -= 4;
> > return nfserr;
> > + }
>
>
> > + xdr->p = savep;
> > + xdr->iov->iov_len = ((char *)resp->xdr.p)
> > + - (char *)resp->xdr.buf->head[0].iov_base;
>
> Where is this coming from? Looks like deep magic to be and would
> benefit from comments, symbolic names and/or a helper.
OK, may be worth a comment even if it's only a temporary thing.
--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
