Re: [PATCH -next] nfs: fix ISO C90 warning

Américo Wang
2009-12-17T10:00:30Z

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

On Thu, Dec 17, 2009 at 7:01 AM, Trond Myklebust
<trond.myklebust@fys.uio.no> wrote:
> On Wed, 2009-12-16 at 23:58 +0100, Thomas Gleixner wrote:
>> On Wed, 16 Dec 2009, Randy Dunlap wrote:
>> > On Wed, 16 Dec 2009 17:40:19 -0500 Trond Myklebust wrote:
>> >
>> > > On Wed, 2009-12-16 at 14:23 -0800, Randy Dunlap wrote:
>> > > > From: Randy Dunlap <randy.dunlap@oracle.com>
>> > > >
>> > > > Fix gcc ISO C90 warning:
>> > > >
>> > > > fs/nfs/callback.c:356: warning: ISO C90 forbids mixed declarations and code
>> > > >
>> > > > Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
>> > > > ---
>> > > >  fs/nfs/callback.c |    2 +-
>> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > > >
>> > > > --- linux-next-20091215.orig/fs/nfs/callback.c
>> > > > +++ linux-next-20091215/fs/nfs/callback.c
>> > > > @@ -352,8 +352,8 @@ static int check_gss_callback_principal(
>> > > >  static int nfs_callback_authenticate(struct svc_rqst *rqstp)
>> > > >  {
>> > > >         struct nfs_client *clp;
>> > > > -       RPC_IFDEBUG(char buf[RPC_MAX_ADDRBUFLEN]);
>> > > >         int ret = SVC_OK;
>> > > > +       RPC_IFDEBUG(char buf[RPC_MAX_ADDRBUFLEN]);
>> > > >
>> > >
>> > > What version of gcc is giving rise to this warning?
>> >
>> > > gcc --version
>> > gcc (GCC) 4.2.1 (SUSE Linux)
>> >
>> > > RPC_IFDEBUG is a macro that either evaluates to its argument, or to
>> > > nothing, depending on whether or not RPC_DEBUG is defined or not. In
>> > > neither case should it evaluate to anything illegal under C90 rules
>> > > afaics.
>> >
>> > Yep.  Odd warning.
>>
>> Not really. If the debug macro evaluates to nothing then you have:
>>
>>     struct nfs_client *clp;
>>     ;
>>     int ret = SVC_OK;
>>
>> So you have a stray semicolon, which is interpreted as an empty code
>> line. That qualifies for the mixed declaration and code case :)
>>
>> I know it's nitpicking, but ...
>
> Ah... I see what you mean.
>
> So really what we should do is just move that semicolon inside the
> macro. That would change the !RPC_DEBUG case to
>
>    struct nfs_client *clp;
>
>    int ret = SVC_OK;
>
> which is 100% legal...
>

Hi,
Check that currently all usages of RPC_DEBUG are the same,
we can replace them all.

What do you think about the attached patch?

Sorry for attaching it, I have a bad mail environment here.

Signed-off-by: WANG Cong <xiyou.wangcong@gmail.com>

[-- Attachment #2: rpc-replace-rpc-ifdebug.diff --]
[-- Type: text/plain, Size: 3004 bytes --]

diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index e50cfa3..4fce0f2 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -152,7 +152,7 @@ lockd(void *vrqstp)
*/
while (!kthread_should_stop()) {
long timeout = MAX_SCHEDULE_TIMEOUT;
- RPC_IFDEBUG(char buf[RPC_MAX_ADDRBUFLEN]);
+ RPCBUF_IFDEBUG(buf);

/* update sv_maxconn if it has changed */
rqstp->rq_server->sv_maxconn = nlm_max_connections;
diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
index 73ab220..e05b6fc 100644
--- a/fs/nfs/callback.c
+++ b/fs/nfs/callback.c
@@ -352,7 +352,7 @@ static int check_gss_callback_principal(struct nfs_client *clp,
static int nfs_callback_authenticate(struct svc_rqst *rqstp)
{
struct nfs_client *clp;
- RPC_IFDEBUG(char buf[RPC_MAX_ADDRBUFLEN]);
+ RPCBUF_IFDEBUG(buf);
int ret = SVC_OK;

/* Don't talk to strangers */
diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index 1c12177..7a84433 100644
--- a/fs/nfsd/nfsfh.c
+++ b/fs/nfsd/nfsfh.c
@@ -90,7 +90,7 @@ static __be32 nfsd_setuser_and_check_port(struct svc_rqst *rqstp,

/* Check if the request originated from a secure port. */
if (!rqstp->rq_secure && (flags & NFSEXP_INSECURE_PORT)) {
- RPC_IFDEBUG(char buf[RPC_MAX_ADDRBUFLEN]);
+ RPCBUF_IFDEBUG(buf);
dprintk(KERN_WARNING
"nfsd: request from insecure port %s!\n",
svc_print_addr(rqstp, buf, sizeof(buf)));
diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h
index c2786f2..13185bb 100644
--- a/include/linux/sunrpc/debug.h
+++ b/include/linux/sunrpc/debug.h
@@ -52,11 +52,11 @@ extern unsigned int nlm_debug;
#ifdef RPC_DEBUG
# define ifdebug(fac) if (unlikely(rpc_debug & RPCDBG_##fac))
# define dfprintk(fac, args...) do { ifdebug(fac) printk(args); } while(0)
-# define RPC_IFDEBUG(x) x
+# define RPCBUF_IFDEBUG(x) char x[RPC_MAX_ADDRBUFLEN]
#else
# define ifdebug(fac) if (0)
# define dfprintk(fac, args...) do ; while (0)
-# define RPC_IFDEBUG(x)
+# define RPCBUF_IFDEBUG(x) char x[0]
#endif

/*
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 870929e..425c4c1 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -228,7 +228,7 @@ static int svc_sendto(struct svc_rqst *rqstp, struct xdr_buf *xdr)
int len = 0;
unsigned long tailoff;
unsigned long headoff;
- RPC_IFDEBUG(char buf[RPC_MAX_ADDRBUFLEN]);
+ RPCBUF_IFDEBUG(buf);

if (rqstp->rq_prot == IPPROTO_UDP) {
struct msghdr msg = {
@@ -808,7 +808,7 @@ static struct svc_xprt *svc_tcp_accept(struct svc_xprt *xprt)
struct socket *newsock;
struct svc_sock *newsvsk;
int err, slen;
- RPC_IFDEBUG(char buf[RPC_MAX_ADDRBUFLEN]);
+ RPCBUF_IFDEBUG(buf);

dprintk("svc: tcp_accept %p sock %p\n", svsk, sock);
if (!sock)
@@ -1410,7 +1410,7 @@ static struct svc_xprt *svc_create_socket(struct svc_serv *serv,
int newlen;
int family;
int val;
- RPC_IFDEBUG(char buf[RPC_MAX_ADDRBUFLEN]);
+ RPCBUF_IFDEBUG(buf);

dprintk("svc: svc_create_socket(%s, %d, %s)\n",
serv->sv_program->pg_name, protocol,