Re: [PATH v6 2/9] nfsd41: handle current stateid in open and close

["J. Bruce Fields" <bfields@fieldses.org>]

On Tue, Jan 17, 2012 at 09:14:47PM +0100, Tigran Mkrtchyan wrote:
> On Tue, Jan 17, 2012 at 8:40 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> > On Sun, Jan 15, 2012 at 05:20:03PM +0100, Tigran Mkrtchyan wrote:
> >> +static void
> >> +put_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
> >> +{
> >> +     if (cstate->minorversion)
> >> +             cstate->current_stateid = stateid;
> >> +}
> > ...
> >> @@ -54,6 +54,7 @@ struct nfsd4_compound_state {
> >>       size_t                  iovlen;
> >>       u32                     minorversion;
> >>       u32                     status;
> >> +     const stateid_t *current_stateid;
> >>  };
> >
> > I'd be more comfortable with passing stateid's by value rather than by
> > reference.
> >
> > Presumably you're only ever pointing into one of the argument or result
> > structures which are currently guaranteed to be around for as long as
> > the compound is processed.
> >
> > But it'd seem safer not to have to worry about the lifetime of the
> > pointed-to data at all, and just copy the stateid--it's only 16 bytes.
> 
> Sure, it's only 16 bytes. Nevertheless, there are no allocation or
> de-allocation of it. It just pointing to an existing stateid.

Well, it's conceivable some day that we could for example take more of a
one-op-at-a-time approach to compound processing and free the arguments
and results from the previous op once we're done processing and xdr
encoding it, in which case continuing to refer to a stateid from the
previous op would be unsafe.

Or maybe some day there will be an operation that sets the current
stateid without also returning it in an argument, in which case we could
be tempted to take a pointer to a field in an object without being
assured how long that object will be around.

Likely?  Maybe not.  I'd still feel safer not having to think about it.

> Should be safe to use pointer. And yes, it's just 16 bytes, but if we can
> avoid to
> copy them why not?
> 
> Of course I can change it if you want to.

I'd prefer that.

--b.