linux-next: nfs build failure

linux-next: nfs build failure

[Stephen Rothwell]

Hi Trond,

Today's linux-next build (powerpc ppc44x_defconfig) failed like this:

fs/nfs/super.c: In function 'nfs_remount':
fs/nfs/super.c:1440: error: 'nfs4_fs_type' undeclared (first use in this function)

Caused by commit 68a2efd9008ee81e72a4d1dcdf228414f5dfa6ff ("NFS:
implement option checking when remounting NFS filesystems (resend)").

I applied the following patch (which may not be correct).
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

>From 9e9b8509043be3ccb6c045df11200ee589e9a42a Mon Sep 17 00:00:00 2001
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 17 Jun 2008 19:23:18 +1000
Subject: [PATCH] nfs fix 1

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>

---
 fs/nfs/super.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 226ae6a..e951972 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1437,7 +1437,10 @@ nfs_remount(struct super_block *sb, int *flags, char *raw_data)
 	 * ones were explicitly specified. Fall back to legacy behavior and
 	 * just return success.
 	 */
-	if ((sb->s_type == &nfs4_fs_type && options4->version == 1) ||
+	if (
+#ifdef CONFIG_NFS_V4
+	    (sb->s_type == &nfs4_fs_type && options4->version == 1) ||
+#endif
 	    (sb->s_type == &nfs_fs_type && options->version >= 1 &&
 	     options->version <= 6))
 		return 0;
-- 
1.5.5.4

Re: linux-next: nfs build failure

[Trond Myklebust]

On Tue, 2008-06-17 at 19:26 +1000, Stephen Rothwell wrote:
> Hi Trond,
> 
> Today's linux-next build (powerpc ppc44x_defconfig) failed like this:
> 
> fs/nfs/super.c: In function 'nfs_remount':
> fs/nfs/super.c:1440: error: 'nfs4_fs_type' undeclared (first use in this function)
> 
> Caused by commit 68a2efd9008ee81e72a4d1dcdf228414f5dfa6ff ("NFS:
> implement option checking when remounting NFS filesystems (resend)").
> 
> I applied the following patch (which may not be correct).

Thanks Stephen! The patch looks correct, but I think I'd still prefer
the following to avoid the #ifdef.

Cheers
  Trond
----------------------------------------------------------------
From: Trond Myklebust <Trond.Myklebust@netapp.com>
Date: Tue, 17 Jun 2008 16:12:00 -0400
NFS: Fix a dependency on CONFIG_NFS_V4 in nfs_remount

Fix the 'nfs4_fs_type' undeclared error in nfs_remount when compiling sans
NFSv4...

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: Jeff Layton <jlayton@redhat.com>
---

 fs/nfs/super.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 39c028e..c8f29ac 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1426,6 +1426,7 @@ nfs_remount(struct super_block *sb, int *flags, char *raw_data)
 	struct nfs_parsed_mount_data *data;
 	struct nfs_mount_data *options = (struct nfs_mount_data *)raw_data;
 	struct nfs4_mount_data *options4 = (struct nfs4_mount_data *)raw_data;
+	u32 nfsvers = nfss->nfs_client->rpc_ops->version;
 
 	/*
 	 * Userspace mount programs that send binary options generally send
@@ -1433,8 +1434,8 @@ nfs_remount(struct super_block *sb, int *flags, char *raw_data)
 	 * ones were explicitly specified. Fall back to legacy behavior and
 	 * just return success.
 	 */
-	if ((sb->s_type == &nfs4_fs_type && options4->version == 1) ||
-	    (sb->s_type == &nfs_fs_type && options->version >= 1 &&
+	if ((nfsvers == 4 && options4->version == 1) ||
+	    (nfsvers <= 3 && options->version >= 1 &&
 	     options->version <= 6))
 		return 0;
 

Re: linux-next: nfs build failure

[Jeff Layton]

On Tue, 17 Jun 2008 16:19:15 -0400
Trond Myklebust <trond.myklebust@fys.uio.no> wrote:

> On Tue, 2008-06-17 at 19:26 +1000, Stephen Rothwell wrote:
> > Hi Trond,
> > 
> > Today's linux-next build (powerpc ppc44x_defconfig) failed like this:
> > 
> > fs/nfs/super.c: In function 'nfs_remount':
> > fs/nfs/super.c:1440: error: 'nfs4_fs_type' undeclared (first use in this function)
> > 
> > Caused by commit 68a2efd9008ee81e72a4d1dcdf228414f5dfa6ff ("NFS:
> > implement option checking when remounting NFS filesystems (resend)").
> > 
> > I applied the following patch (which may not be correct).
> 
> Thanks Stephen! The patch looks correct, but I think I'd still prefer
> the following to avoid the #ifdef.
> 
> Cheers
>   Trond
> ----------------------------------------------------------------
> From: Trond Myklebust <Trond.Myklebust@netapp.com>
> Date: Tue, 17 Jun 2008 16:12:00 -0400
> NFS: Fix a dependency on CONFIG_NFS_V4 in nfs_remount
> 
> Fix the 'nfs4_fs_type' undeclared error in nfs_remount when compiling sans
> NFSv4...
> 
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> Cc: Jeff Layton <jlayton@redhat.com>
> ---
> 
>  fs/nfs/super.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 39c028e..c8f29ac 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -1426,6 +1426,7 @@ nfs_remount(struct super_block *sb, int *flags, char *raw_data)
>  	struct nfs_parsed_mount_data *data;
>  	struct nfs_mount_data *options = (struct nfs_mount_data *)raw_data;
>  	struct nfs4_mount_data *options4 = (struct nfs4_mount_data *)raw_data;
> +	u32 nfsvers = nfss->nfs_client->rpc_ops->version;
>  
>  	/*
>  	 * Userspace mount programs that send binary options generally send
> @@ -1433,8 +1434,8 @@ nfs_remount(struct super_block *sb, int *flags, char *raw_data)
>  	 * ones were explicitly specified. Fall back to legacy behavior and
>  	 * just return success.
>  	 */
> -	if ((sb->s_type == &nfs4_fs_type && options4->version == 1) ||
> -	    (sb->s_type == &nfs_fs_type && options->version >= 1 &&
> +	if ((nfsvers == 4 && options4->version == 1) ||
> +	    (nfsvers <= 3 && options->version >= 1 &&
>  	     options->version <= 6))
>  		return 0;
>  
> 
> 

ACK...that looks better to me too.

-- 
Jeff Layton <jlayton@redhat.com>

Re: linux-next: nfs build failure

[Stephen Rothwell]

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

Hi Trond,

On Tue, 17 Jun 2008 16:19:15 -0400 Trond Myklebust <trond.myklebust@fys.uio.no> wrote:
>
> Thanks Stephen! The patch looks correct, but I think I'd still prefer
> the following to avoid the #ifdef.

Its your tree, I just hack it so it will build for me.  :-)

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

linux-next: nfs build failure

[Stephen Rothwell]

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

Hi Trond,

Today's linux-next build (powerpc ppc64_defconfig) failed like this:

fs/built-in.o: In function `.nfs_parse_ip_address':
super.c:(.text+0xe2f08): undefined reference to `.__ipv6_addr_type'

Some CONFIG_IPV6 protections are needed ...

I reverted commit 09491a874d4f9a8676567bc58bce9dec9539740d ("NFS: handle
interface identifiers in incoming IPv6 addresses").

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: linux-next: nfs build failure

[Trond Myklebust]

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

On Wed, 2008-06-18 at 12:52 +1000, Stephen Rothwell wrote:
> Hi Trond,
> 
> Today's linux-next build (powerpc ppc64_defconfig) failed like this:
> 
> fs/built-in.o: In function `.nfs_parse_ip_address':
> super.c:(.text+0xe2f08): undefined reference to `.__ipv6_addr_type'
> 
> Some CONFIG_IPV6 protections are needed ...
> 
> I reverted commit 09491a874d4f9a8676567bc58bce9dec9539740d ("NFS: handle
> interface identifiers in incoming IPv6 addresses").

I suggest something like the attached patch. Comments Chuck?

Trond



[-- Attachment #2: linux-2.6.26-023-dont_parse_ipv6_if_no_config_ipv6.dif --]
[-- Type: message/rfc822, Size: 3160 bytes --]

From: Trond Myklebust <Trond.Myklebust@netapp.com>
Subject: NFS: Don't allow IPv6 addresses if CONFIG_IPv6 isn't set
Date: Wed, 18 Jun 2008 16:04:22 -0400
Message-ID: <1213820042.8233.22.camel@localhost>


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2.1: Type: text/plain, Size: 2148 bytes --]

From: Trond Myklebust <Trond.Myklebust@netapp.com>
Subject: NFS: Don't allow IPv6 addresses if CONFIG_IPv6 isn't set
Date: Wed, 18 Jun 2008 16:04:22 -0400
Message-ID: <1213820042.8233.22.camel@localhost>

Fixes a compile failure in fs/nfs/super.c

Also fix the compiler warnings:
  fs/nfs/super.c:721: warning: field width should have type ‘int’, but
  		      argument 2 has type ‘size_t’
  fs/nfs/super.c:809:3: warning: returning void-valued expression
  fs/nfs/super.c:811:3: warning: returning void-valued expression

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 fs/nfs/super.c |   19 ++++++++++++++-----
 1 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index e62820c..6a47bc3 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -718,10 +718,10 @@ static void nfs_parse_ipv4_address(char *string, size_t str_len,
 	struct sockaddr_in *sin = (struct sockaddr_in *)sap;
 	u8 *addr = (u8 *)&sin->sin_addr.s_addr;
 
-	dfprintk(MOUNT, "NFS: parsing IPv4 address %*s\n",
-			str_len, string);
-
 	if (str_len <= INET_ADDRSTRLEN) {
+		dfprintk(MOUNT, "NFS: parsing IPv4 address %*s\n",
+				(int)str_len, string);
+
 		sin->sin_family = AF_INET;
 		*addr_len = sizeof(*sin);
 		if (in4_pton(string, str_len, addr, '\0', NULL))
@@ -732,6 +732,7 @@ static void nfs_parse_ipv4_address(char *string, size_t str_len,
 	*addr_len = 0;
 }
 
+#ifdef CONFIG_IPV6
 static void nfs_parse_ipv6_scope_id(const char *string, const size_t str_len,
 				    const char *delim,
 				    struct sockaddr_in6 *sin6)
@@ -787,6 +788,14 @@ static void nfs_parse_ipv6_address(char *string, size_t str_len,
 	sap->sa_family = AF_UNSPEC;
 	*addr_len = 0;
 }
+#else
+static void nfs_parse_ipv6_address(char *string, size_t str_len,
+				   struct sockaddr *sap, size_t *addr_len)
+{
+	sap->sa_family = AF_UNSPEC;
+	*addr_len = 0;
+}
+#endif
 
 /*
  * Construct a sockaddr based on the contents of a string that contains
@@ -806,9 +815,9 @@ static void nfs_parse_ip_address(char *string, size_t str_len,
 			colons++;
 
 	if (colons >= 2)
-		return nfs_parse_ipv6_address(string, str_len, sap, addr_len);
+		nfs_parse_ipv6_address(string, str_len, sap, addr_len);
 	else
-		return nfs_parse_ipv4_address(string, str_len, sap, addr_len);
+		nfs_parse_ipv4_address(string, str_len, sap, addr_len);
 }
 
 /*

Re: linux-next: nfs build failure

[Chuck Lever]

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

Trond Myklebust wrote:
> On Wed, 2008-06-18 at 12:52 +1000, Stephen Rothwell wrote:
>> Hi Trond,
>>
>> Today's linux-next build (powerpc ppc64_defconfig) failed like this:
>>
>> fs/built-in.o: In function `.nfs_parse_ip_address':
>> super.c:(.text+0xe2f08): undefined reference to `.__ipv6_addr_type'
>>
>> Some CONFIG_IPV6 protections are needed ...
>>
>> I reverted commit 09491a874d4f9a8676567bc58bce9dec9539740d ("NFS: handle
>> interface identifiers in incoming IPv6 addresses").
> 
> I suggest something like the attached patch. Comments Chuck?
> 
> Trond
> 
> 
> 
> 
> ------------------------------------------------------------------------
> 
> Subject:
> NFS: Don't allow IPv6 addresses if CONFIG_IPv6 isn't set
> From:
> Trond Myklebust <Trond.Myklebust@netapp.com>
> Date:
> Wed, 18 Jun 2008 16:04:22 -0400
> 
> 
> Fixes a compile failure in fs/nfs/super.c
> 
> Also fix the compiler warnings:
>   fs/nfs/super.c:721: warning: field width should have type ‘int’, but
>   		      argument 2 has type ‘size_t’

I just saw this warning yesterday, and was about to send a fix for it. 
Sorry for the delay, but I had a power supply go south on me today.

>   fs/nfs/super.c:809:3: warning: returning void-valued expression
>   fs/nfs/super.c:811:3: warning: returning void-valued expression

But I've never seen that one.  Very strange how the warnings vary 
depending on what platform you compile to.

> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
> 
>  fs/nfs/super.c |   19 ++++++++++++++-----
>  1 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index e62820c..6a47bc3 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -718,10 +718,10 @@ static void nfs_parse_ipv4_address(char *string, size_t str_len,
>  	struct sockaddr_in *sin = (struct sockaddr_in *)sap;
>  	u8 *addr = (u8 *)&sin->sin_addr.s_addr;
>  
> -	dfprintk(MOUNT, "NFS: parsing IPv4 address %*s\n",
> -			str_len, string);
> -
>  	if (str_len <= INET_ADDRSTRLEN) {
> +		dfprintk(MOUNT, "NFS: parsing IPv4 address %*s\n",
> +				(int)str_len, string);
> +

You need the same fix in nfs_parse_ipv6_address().

>  		sin->sin_family = AF_INET;
>  		*addr_len = sizeof(*sin);
>  		if (in4_pton(string, str_len, addr, '\0', NULL))
> @@ -732,6 +732,7 @@ static void nfs_parse_ipv4_address(char *string, size_t str_len,
>  	*addr_len = 0;
>  }
>  
> +#ifdef CONFIG_IPV6
>  static void nfs_parse_ipv6_scope_id(const char *string, const size_t str_len,
>  				    const char *delim,
>  				    struct sockaddr_in6 *sin6)
> @@ -787,6 +788,14 @@ static void nfs_parse_ipv6_address(char *string, size_t str_len,
>  	sap->sa_family = AF_UNSPEC;
>  	*addr_len = 0;
>  }
> +#else
> +static void nfs_parse_ipv6_address(char *string, size_t str_len,
> +				   struct sockaddr *sap, size_t *addr_len)
> +{
> +	sap->sa_family = AF_UNSPEC;
> +	*addr_len = 0;
> +}
> +#endif

Instead of creating a separate function, you could just wrap everything 
in nfs_parse_ipv6_address() but these two lines with #ifdef CONFIG_IPV6 
/ #endif.

>  
>  /*
>   * Construct a sockaddr based on the contents of a string that contains
> @@ -806,9 +815,9 @@ static void nfs_parse_ip_address(char *string, size_t str_len,
>  			colons++;
>  
>  	if (colons >= 2)
> -		return nfs_parse_ipv6_address(string, str_len, sap, addr_len);
> +		nfs_parse_ipv6_address(string, str_len, sap, addr_len);
>  	else
> -		return nfs_parse_ipv4_address(string, str_len, sap, addr_len);
> +		nfs_parse_ipv4_address(string, str_len, sap, addr_len);
>  }
>  
>  /*

/me slaps forehead.  D'oh!

[-- Attachment #2: chuck_lever.vcf --]
[-- Type: text/x-vcard, Size: 259 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
title:Principal Member of Staff
tel;work:+1 248 614 5091
x-mozilla-html:FALSE
version:2.1
end:vcard

Re: linux-next: nfs build failure

[Trond Myklebust]

On Wed, 2008-06-18 at 16:37 -0400, Chuck Lever wrote:
> Trond Myklebust wrote:
> > On Wed, 2008-06-18 at 12:52 +1000, Stephen Rothwell wrote:
> >> Hi Trond,
> >>
> >> Today's linux-next build (powerpc ppc64_defconfig) failed like this:
> >>
> >> fs/built-in.o: In function `.nfs_parse_ip_address':
> >> super.c:(.text+0xe2f08): undefined reference to `.__ipv6_addr_type'
> >>
> >> Some CONFIG_IPV6 protections are needed ...
> >>
> >> I reverted commit 09491a874d4f9a8676567bc58bce9dec9539740d ("NFS: handle
> >> interface identifiers in incoming IPv6 addresses").
> > 
> > I suggest something like the attached patch. Comments Chuck?
> > 
> > Trond
> > 
> > 
> > 
> > 
> > ------------------------------------------------------------------------
> > 
> > Subject:
> > NFS: Don't allow IPv6 addresses if CONFIG_IPv6 isn't set
> > From:
> > Trond Myklebust <Trond.Myklebust@netapp.com>
> > Date:
> > Wed, 18 Jun 2008 16:04:22 -0400
> > 
> > 
> > Fixes a compile failure in fs/nfs/super.c
> > 
> > Also fix the compiler warnings:
> >   fs/nfs/super.c:721: warning: field width should have type ‘int’, but
> >   		      argument 2 has type ‘size_t’
> 
> I just saw this warning yesterday, and was about to send a fix for it. 
> Sorry for the delay, but I had a power supply go south on me today.
> 
> >   fs/nfs/super.c:809:3: warning: returning void-valued expression
> >   fs/nfs/super.c:811:3: warning: returning void-valued expression
> 
> But I've never seen that one.  Very strange how the warnings vary 
> depending on what platform you compile to.
> 
> > Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> > ---
> > 
> >  fs/nfs/super.c |   19 ++++++++++++++-----
> >  1 files changed, 14 insertions(+), 5 deletions(-)
> > 
> > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > index e62820c..6a47bc3 100644
> > --- a/fs/nfs/super.c
> > +++ b/fs/nfs/super.c
> > @@ -718,10 +718,10 @@ static void nfs_parse_ipv4_address(char *string, size_t str_len,
> >  	struct sockaddr_in *sin = (struct sockaddr_in *)sap;
> >  	u8 *addr = (u8 *)&sin->sin_addr.s_addr;
> >  
> > -	dfprintk(MOUNT, "NFS: parsing IPv4 address %*s\n",
> > -			str_len, string);
> > -
> >  	if (str_len <= INET_ADDRSTRLEN) {
> > +		dfprintk(MOUNT, "NFS: parsing IPv4 address %*s\n",
> > +				(int)str_len, string);
> > +
> 
> You need the same fix in nfs_parse_ipv6_address().

There is no such printk in nfs_parse_ipv6_address().

> >  		sin->sin_family = AF_INET;
> >  		*addr_len = sizeof(*sin);
> >  		if (in4_pton(string, str_len, addr, '\0', NULL))
> > @@ -732,6 +732,7 @@ static void nfs_parse_ipv4_address(char *string, size_t str_len,
> >  	*addr_len = 0;
> >  }
> >  
> > +#ifdef CONFIG_IPV6
> >  static void nfs_parse_ipv6_scope_id(const char *string, const size_t str_len,
> >  				    const char *delim,
> >  				    struct sockaddr_in6 *sin6)
> > @@ -787,6 +788,14 @@ static void nfs_parse_ipv6_address(char *string, size_t str_len,
> >  	sap->sa_family = AF_UNSPEC;
> >  	*addr_len = 0;
> >  }
> > +#else
> > +static void nfs_parse_ipv6_address(char *string, size_t str_len,
> > +				   struct sockaddr *sap, size_t *addr_len)
> > +{
> > +	sap->sa_family = AF_UNSPEC;
> > +	*addr_len = 0;
> > +}
> > +#endif
> 
> Instead of creating a separate function, you could just wrap everything 
> in nfs_parse_ipv6_address() but these two lines with #ifdef CONFIG_IPV6 
> / #endif.

Getting rid of those #ifdef eyesores inside function definitions is
worth the extra few lines.

Trond

--
To unsubscribe from this list: send the line "unsubscribe linux-next" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: linux-next: nfs build failure

[Randy Dunlap]

On Wed, 18 Jun 2008 17:15:13 -0400 Trond Myklebust wrote:

> On Wed, 2008-06-18 at 16:37 -0400, Chuck Lever wrote:
> > Trond Myklebust wrote:
> > > On Wed, 2008-06-18 at 12:52 +1000, Stephen Rothwell wrote:
> > >> Hi Trond,
> > >>
> > >> Today's linux-next build (powerpc ppc64_defconfig) failed like this:
> > >>
> > >> fs/built-in.o: In function `.nfs_parse_ip_address':
> > >> super.c:(.text+0xe2f08): undefined reference to `.__ipv6_addr_type'
> > >>
> > >> Some CONFIG_IPV6 protections are needed ...
> > >>
> > >> I reverted commit 09491a874d4f9a8676567bc58bce9dec9539740d ("NFS: handle
> > >> interface identifiers in incoming IPv6 addresses").
> > > 
> > > I suggest something like the attached patch. Comments Chuck?
> > > 
> > > Trond
> > > 
> > > 
> > > 
> > > 
> > > ------------------------------------------------------------------------
> > > 
> > > Subject:
> > > NFS: Don't allow IPv6 addresses if CONFIG_IPv6 isn't set
> > > From:
> > > Trond Myklebust <Trond.Myklebust@netapp.com>
> > > Date:
> > > Wed, 18 Jun 2008 16:04:22 -0400
> > > 
> > > 
> > > Fixes a compile failure in fs/nfs/super.c
> > > 
> > > Also fix the compiler warnings:
> > >   fs/nfs/super.c:721: warning: field width should have type ‘int’, but
> > >   		      argument 2 has type ‘size_t’
> > 
> > I just saw this warning yesterday, and was about to send a fix for it. 
> > Sorry for the delay, but I had a power supply go south on me today.
> > 
> > >   fs/nfs/super.c:809:3: warning: returning void-valued expression
> > >   fs/nfs/super.c:811:3: warning: returning void-valued expression
> > 
> > But I've never seen that one.  Very strange how the warnings vary 
> > depending on what platform you compile to.
> > 
> > > Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> > > ---
> > > 
> > >  fs/nfs/super.c |   19 ++++++++++++++-----
> > >  1 files changed, 14 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > > index e62820c..6a47bc3 100644
> > > --- a/fs/nfs/super.c
> > > +++ b/fs/nfs/super.c
> > > @@ -718,10 +718,10 @@ static void nfs_parse_ipv4_address(char *string, size_t str_len,
> > >  	struct sockaddr_in *sin = (struct sockaddr_in *)sap;
> > >  	u8 *addr = (u8 *)&sin->sin_addr.s_addr;
> > >  
> > > -	dfprintk(MOUNT, "NFS: parsing IPv4 address %*s\n",
> > > -			str_len, string);
> > > -
> > >  	if (str_len <= INET_ADDRSTRLEN) {
> > > +		dfprintk(MOUNT, "NFS: parsing IPv4 address %*s\n",
> > > +				(int)str_len, string);
> > > +
> > 
> > You need the same fix in nfs_parse_ipv6_address().
> 
> There is no such printk in nfs_parse_ipv6_address().

There is such a 'dfprintk()' in that function in today's linux-next tree,
with the same warning.


> > >  		sin->sin_family = AF_INET;
> > >  		*addr_len = sizeof(*sin);
> > >  		if (in4_pton(string, str_len, addr, '\0', NULL))
> > > @@ -732,6 +732,7 @@ static void nfs_parse_ipv4_address(char *string, size_t str_len,
> > >  	*addr_len = 0;
> > >  }
> > >  
> > > +#ifdef CONFIG_IPV6
> > >  static void nfs_parse_ipv6_scope_id(const char *string, const size_t str_len,
> > >  				    const char *delim,
> > >  				    struct sockaddr_in6 *sin6)
> > > @@ -787,6 +788,14 @@ static void nfs_parse_ipv6_address(char *string, size_t str_len,
> > >  	sap->sa_family = AF_UNSPEC;
> > >  	*addr_len = 0;
> > >  }
> > > +#else
> > > +static void nfs_parse_ipv6_address(char *string, size_t str_len,
> > > +				   struct sockaddr *sap, size_t *addr_len)
> > > +{
> > > +	sap->sa_family = AF_UNSPEC;
> > > +	*addr_len = 0;
> > > +}
> > > +#endif
> > 
> > Instead of creating a separate function, you could just wrap everything 
> > in nfs_parse_ipv6_address() but these two lines with #ifdef CONFIG_IPV6 
> > / #endif.
> 
> Getting rid of those #ifdef eyesores inside function definitions is
> worth the extra few lines.


---
~Randy
Linux Plumbers Conference, 17-19 September 2008, Portland, Oregon USA
http://linuxplumbersconf.org/
--
To unsubscribe from this list: send the line "unsubscribe linux-next" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: linux-next: nfs build failure

[Trond Myklebust]

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

On Wed, 2008-06-18 at 14:19 -0700, Randy Dunlap wrote:

> > > 
> > > You need the same fix in nfs_parse_ipv6_address().
> > 
> > There is no such printk in nfs_parse_ipv6_address().
> 
> There is such a 'dfprintk()' in that function in today's linux-next tree,
> with the same warning.

Gack, you're right. In fact that shows up another bug: #ifdef
CONFIG_IPV6 isn't sufficient. The reason the above dfprintk wasn't
triggering for me was that you also need to check for
CONFIG_IPV6_MODULE.

Sigh. Revised patch is attached...

  Trond

[-- Attachment #2: linux-2.6.26-023-dont_parse_ipv6_if_no_config_ipv6.dif --]
[-- Type: message/rfc822, Size: 4028 bytes --]

From: Trond Myklebust <Trond.Myklebust@netapp.com>
Subject: NFS: Don't allow IPv6 addresses if CONFIG_IPv6 isn't set
Date: Wed, 18 Jun 2008 16:04:22 -0400
Message-ID: <1213827426.25182.61.camel@localhost>


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2.1: Type: text/plain, Size: 2790 bytes --]

From: Trond Myklebust <Trond.Myklebust@netapp.com>
Subject: NFS: Don't allow IPv6 addresses if CONFIG_IPv6 isn't set
Date: Wed, 18 Jun 2008 16:04:22 -0400
Message-ID: <1213827426.25182.61.camel@localhost>

Fixes a compile failure in fs/nfs/super.c

Also fix the compiler warnings:
  fs/nfs/super.c:721: warning: field width should have type ‘int’, but
  		      argument 2 has type ‘size_t’
  fs/nfs/super.c:776: warning: field width should have type ‘int’, but
  		      argument 2 has type ‘size_t’
  fs/nfs/super.c:809:3: warning: returning void-valued expression
  fs/nfs/super.c:811:3: warning: returning void-valued expression

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 fs/nfs/super.c |   25 +++++++++++++++++--------
 1 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index e62820c..1d98e95 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -718,10 +718,10 @@ static void nfs_parse_ipv4_address(char *string, size_t str_len,
 	struct sockaddr_in *sin = (struct sockaddr_in *)sap;
 	u8 *addr = (u8 *)&sin->sin_addr.s_addr;
 
-	dfprintk(MOUNT, "NFS: parsing IPv4 address %*s\n",
-			str_len, string);
-
 	if (str_len <= INET_ADDRSTRLEN) {
+		dfprintk(MOUNT, "NFS: parsing IPv4 address %*s\n",
+				(int)str_len, string);
+
 		sin->sin_family = AF_INET;
 		*addr_len = sizeof(*sin);
 		if (in4_pton(string, str_len, addr, '\0', NULL))
@@ -732,6 +732,7 @@ static void nfs_parse_ipv4_address(char *string, size_t str_len,
 	*addr_len = 0;
 }
 
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
 static void nfs_parse_ipv6_scope_id(const char *string, const size_t str_len,
 				    const char *delim,
 				    struct sockaddr_in6 *sin6)
@@ -772,10 +773,10 @@ static void nfs_parse_ipv6_address(char *string, size_t str_len,
 	u8 *addr = (u8 *)&sin6->sin6_addr.in6_u;
 	const char *delim;
 
-	dfprintk(MOUNT, "NFS: parsing IPv6 address %*s\n",
-			str_len, string);
-
 	if (str_len <= INET6_ADDRSTRLEN) {
+		dfprintk(MOUNT, "NFS: parsing IPv6 address %*s\n",
+				(int)str_len, string);
+
 		sin6->sin6_family = AF_INET6;
 		*addr_len = sizeof(*sin6);
 		if (in6_pton(string, str_len, addr, SCOPE_DELIMITER, &delim)) {
@@ -787,6 +788,14 @@ static void nfs_parse_ipv6_address(char *string, size_t str_len,
 	sap->sa_family = AF_UNSPEC;
 	*addr_len = 0;
 }
+#else
+static void nfs_parse_ipv6_address(char *string, size_t str_len,
+				   struct sockaddr *sap, size_t *addr_len)
+{
+	sap->sa_family = AF_UNSPEC;
+	*addr_len = 0;
+}
+#endif
 
 /*
  * Construct a sockaddr based on the contents of a string that contains
@@ -806,9 +815,9 @@ static void nfs_parse_ip_address(char *string, size_t str_len,
 			colons++;
 
 	if (colons >= 2)
-		return nfs_parse_ipv6_address(string, str_len, sap, addr_len);
+		nfs_parse_ipv6_address(string, str_len, sap, addr_len);
 	else
-		return nfs_parse_ipv4_address(string, str_len, sap, addr_len);
+		nfs_parse_ipv4_address(string, str_len, sap, addr_len);
 }
 
 /*