2010-06-15 17:32:33

by Trond Myklebust

[permalink] [raw]
Subject: RE: [PATCH 02/15] NFSv4.1: Clean up nfs4_setup_sequence

On Tue, 2010-06-15 at 09:50 -0700, Gilliam, PaulX J wrote:
> > -----Original Message-----
> > From: [email protected] [mailto:[email protected]] On Behalf Of Trond Myklebust
> > Sent: Monday, June 14, 2010 2:51 PM
> > To: [email protected]
> > Subject: [PATCH 02/15] NFSv4.1: Clean up nfs4_setup_sequence
> >
> > Firstly, there is little point in first zeroing out the entire struct
> > nfs4_sequence_res, and then initialising all fields save one. Just
> > initialise the last field to zero...
>
> The reason one may want to zero out the entire struct is that in the future, someone may add elements to the struct. In that case,
> memset(res, 0, sizeof(*res));
> would not have to be changed, and any new elements will "automatically" be initialized to a known value.
>
> Just a thought.

That assumes this is a structure that is likely to change and/or be
extended in the future, which is unlikely since the NFSv4.1 protocol
specification is complete and the SEQUENCE results are fully contained
in the current set of fields.

It also assumes that '0' would be a desirable default value for these
new fields.

Trond



2010-06-15 17:53:40

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 02/15] NFSv4.1: Clean up nfs4_setup_sequence

On Tue, Jun 15, 2010 at 01:32:21PM -0400, Trond Myklebust wrote:
> On Tue, 2010-06-15 at 09:50 -0700, Gilliam, PaulX J wrote:
> > > -----Original Message-----
> > > From: [email protected] [mailto:[email protected]] On Behalf Of Trond Myklebust
> > > Sent: Monday, June 14, 2010 2:51 PM
> > > To: [email protected]
> > > Subject: [PATCH 02/15] NFSv4.1: Clean up nfs4_setup_sequence
> > >
> > > Firstly, there is little point in first zeroing out the entire struct
> > > nfs4_sequence_res, and then initialising all fields save one. Just
> > > initialise the last field to zero...
> >
> > The reason one may want to zero out the entire struct is that in the future, someone may add elements to the struct. In that case,
> > memset(res, 0, sizeof(*res));
> > would not have to be changed, and any new elements will "automatically" be initialized to a known value.
> >
> > Just a thought.
>
> That assumes this is a structure that is likely to change and/or be
> extended in the future, which is unlikely since the NFSv4.1 protocol
> specification is complete and the SEQUENCE results are fully contained
> in the current set of fields.
>
> It also assumes that '0' would be a desirable default value for these
> new fields.

Also, the memset's/kzalloc's mean that "git grep <structure field name>"
doesn't turn up the initialization of the field. Not a huge deal, but
it trips me up sometimes.

--b.