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

[AmÃrico Wang]

On Thu, Dec 17, 2009 at 7:01 AM, Trond Myklebust
<trond.myklebust@xxxxxxxxxx> 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@xxxxxxxxxx>
>> > > >
>> > > > 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@xxxxxxxxxx>
>> > > > ---
>> > > > Â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@xxxxxxxxx>
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,