Re: [PATCH 02/10] NFS: Use C99 struct initializers in nfs4proc.c

[Chuck Lever <chuck.lever@oracle.com>]

[-- Attachment #1: Type: text/plain, Size: 3313 bytes --]

Trond Myklebust wrote:
> On Thu, 2007-05-31 at 11:14 -0400, Chuck Lever wrote:
>> Sorry for the delay in responding.  See below.
>>
>> Trond Myklebust wrote:
>>> On Tue, 2007-05-29 at 10:10 -0400, Chuck Lever wrote:
>>>> Trond Myklebust wrote:
>>>>> On Fri, 2007-05-25 at 17:41 -0400, Chuck Lever wrote:
>>>>>> Follow kernel coding conventions.
>>>>>>
>>>>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>>>>> ---
>>>>>>
>>>>>>  fs/nfs/nfs4_fs.h  |    5 ++++
>>>>>>  fs/nfs/nfs4proc.c |   70 ++++++++++++++++++++++++++++-------------------------
>>>>>>  2 files changed, 42 insertions(+), 33 deletions(-)
>>>>>>
>>>>>> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
>>>>>> index cf3a17e..1a526c4 100644
>>>>>> --- a/fs/nfs/nfs4_fs.h
>>>>>> +++ b/fs/nfs/nfs4_fs.h
>>>>>> @@ -145,6 +145,11 @@ struct nfs4_exception {
>>>>>>  	int retry;
>>>>>>  };
>>>>>>  
>>>>>> +#define DECLARE_NFS4_EXCEPTION(name)				\
>>>>>> +	struct nfs4_exception name = {				\
>>>>>> +		.timeout	= 0,				\
>>>>>> +	}
>>>>> Why would we want this? The existing code uses bog standard C99
>>>>> initialisers. There is no need to explicitly set .timeout to zero.
>>>> Using
>>>>
>>>>    struct nfs4_exception name = { };
>>>>
>>>> causes compiler complaints.  One correct way to fix this is to set at 
>>>> least one field in the structure, and setting that field to zero clears 
>>>> the other one by default.
>>> I can't find anything in Harbison & Steele stating that name = {} is
>>> illegal. All they say is that "if there are fewer initializers than
>>> there are structure components, the remaining components are initialized
>>> to their default initial values.".
>> The exact compiler warning is this:
>>
>> /home/cel/src/linux/main/fs/nfs/nfs4proc.c: In function 
>> ‘nfs4_do_open_reclaim’:
>> /home/cel/src/linux/main/fs/nfs/nfs4proc.c:496: warning: missing initializer
>> /home/cel/src/linux/main/fs/nfs/nfs4proc.c:496: warning: (near 
>> initialization for ‘exception.timeout’)
>>
>>> Anyhow, if you really do have to set one component, then a much less
>>> verbose form would be to write "name = {0}" There is no need for the
>>> macro.
>> Yes, that is less verbose.  But it doesn't eliminate the warning, which 
>> becomes this:
>>
>> /home/cel/src/linux/main/fs/nfs/nfs4proc.c: In function 
>> ‘nfs4_do_open_reclaim’:
>> /home/cel/src/linux/main/fs/nfs/nfs4proc.c:496: warning: missing initializer
>> /home/cel/src/linux/main/fs/nfs/nfs4proc.c:496: warning: (near 
>> initialization for ‘exception.retry’)
>>
>> So a complete structure initializer is necessary to eliminate both warnings.
> 
> Sorry, but to me this sounds like a compiler bug. See the above quote,
> read the structure initialisation examples in Harbison & Steele (I know
> you have a copy), and then tell me how this would be any different.

Whether or not the extraneous warning is a compiler bug, I had thought 
it was bad form not to use ".field = foo," when initializing a 
structure.  Are there exceptions to that guideline?  Sadly 
Documentation/CodingStyle is silent on this issue.

It seems to me that specifying "{ 0 };" today will encourage later 
changes that would replace it with "{ 0, foo };" instead of the 
preferred use of ".field = foo,".

[-- Attachment #2: chuck.lever.vcf --]
[-- Type: text/x-vcard, Size: 327 bytes --]

begin:vcard
fn:Chuck Lever
n:Lever;Chuck
org:Oracle Corporation;Corporate Architecture: Linux Projects Group
adr:;;1015 Granger Avenue;Ann Arbor;MI;48104;USA
email;internet:chuck dot lever at nospam oracle dot com
title:Principal Member of Staff
tel;work:+1 248 614 5091
x-mozilla-html:FALSE
version:2.1
end:vcard


[-- Attachment #3: Type: text/plain, Size: 286 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

[-- Attachment #4: Type: text/plain, Size: 140 bytes --]

_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs