Re: [PATCH 03/14] nfsd4: remove redundant "if" in nfs4_preprocess_stateid_op

[Benny Halevy <bhalevy@panasas.com>]

On Mar. 11, 2009, 3:26 +0200, Yang Hongyang <yanghy@cn.fujitsu.com> wrote:
> J. Bruce Fields wrote:
>> On Wed, Mar 11, 2009 at 09:12:46AM +0800, Yang Hongyang wrote:
>>> J. Bruce Fields wrote:
>>>> From: J. Bruce Fields <bfields@citi.umich.edu>
>>>>
>>>> Note that we exit this first big "if" with stp == NULL iff we took the
>>>                                                                                         ^^ typo
>> The "iff"?  That's short for "if and only if".  I can try to get out of
>> the "iff" habit if it's too obscure.
> 
> Sorry for mis-understanding,My pool englishT_T Maybe "if and only if"
> are more understandable.

Although using this iffy math/cs school lingo does not seem too obscure
to me (one can find it easily on the net, e.g.
http://www.tfd.com/?Word=iff), as a non-native English speaker I'd be
inclined to simplify commentary language as much as possible and
refrain from using acronyms if they are not obvious to the linux
developers community.

Benny

> 
>> --b.
>>
>>>> first branch; therefore, the second "if" is redundant, and we can just
>>>> combine the two, simplifying the logic.
>>>>
>>>> Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
>>>> ---
>>>>  fs/nfsd/nfs4state.c |   19 ++++++++-----------
>>>>  1 files changed, 8 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>>>> index d6ca2be..16fcb65 100644
>>>> --- a/fs/nfsd/nfs4state.c
>>>> +++ b/fs/nfsd/nfs4state.c
>>>> @@ -2087,6 +2087,14 @@ nfs4_preprocess_stateid_op(struct svc_fh *current_fh, stateid_t *stateid, int fl
>>>>  		status = check_stateid_generation(stateid, stidp);
>>>>  		if (status)
>>>>  			goto out;
>>>> +		status = nfs4_check_delegmode(dp, flags);
>>>> +		if (status)
>>>> +			goto out;
>>>> +		renew_client(dp->dl_client);
>>>> +		if (flags & DELEG_RET)
>>>> +			unhash_delegation(dp);
>>>> +		if (filpp)
>>>> +			*filpp = dp->dl_vfs_file;
>>>>  	} else { /* open or lock stateid */
>>>>  		stp = find_stateid(stateid, flags);
>>>>  		if (!stp) {
>>>> @@ -2101,23 +2109,12 @@ nfs4_preprocess_stateid_op(struct svc_fh *current_fh, stateid_t *stateid, int fl
>>>>  		status = check_stateid_generation(stateid, stidp);
>>>>  		if (status)
>>>>  			goto out;
>>>> -	}
>>>> -	if (stp) {
>>>>  		status = nfs4_check_openmode(stp, flags);
>>>>  		if (status)
>>>>  			goto out;
>>>>  		renew_client(stp->st_stateowner->so_client);
>>>>  		if (filpp)
>>>>  			*filpp = stp->st_vfs_file;
>>>> -	} else {
>>>> -		status = nfs4_check_delegmode(dp, flags);
>>>> -		if (status)
>>>> -			goto out;
>>>> -		renew_client(dp->dl_client);
>>>> -		if (flags & DELEG_RET)
>>>> -			unhash_delegation(dp);
>>>> -		if (filpp)
>>>> -			*filpp = dp->dl_vfs_file;
>>>>  	}
>>>>  	status = nfs_ok;
>>>>  out:
>>> -- 
>>> Regards
>>> Yang Hongyang
>>
> 
>