[PATCH 00/16] ARM: mostly harmless gcc warnings

[PATCH 00/16] ARM: mostly harmless gcc warnings

[Arnd Bergmann]

In my attempt to weed out all warnings in ARM defconfigs, this
is the latest installment of patches. Ideally these could all
go through the respective subsystem maintainer trees, but
for any patches I don't hear anything back on, I will just
merge them through the arm-soc tree.

	Arnd

Arnd Bergmann (16):
  ARM: warnings in arch/arm/include/asm/uaccess.h
  ARM: binfmt_flat: unused variable 'persistent'
  SCSI: ARM: ncr5380/oak uses no interrupts
  SCSI: ARM: make fas216_dumpinfo function conditional
  vfs: bogus warnings in fs/namei.c
  mm/slob: use min_t() to compare ARCH_SLAB_MINALIGN
  cgroup: fix warning when building without any subsys
  ipvs: fix ip_vs_set_timeout debug messages
  USB: EHCI: mark ehci_orion_conf_mbus_windows __devinit
  clk: don't mark clkdev_add_table as init
  pcmcia: sharpsl: don't discard sharpsl_pcmcia_ops
  video: mark nuc900fb_map_video_memory as __devinit
  ARM: be really quiet when building with 'make -s'
  ARM: pxa: armcore: fix PCI PIO warnings
  spi/s3c64xx: use correct dma_transfer_direction type
  ARM: pass -marm to gcc by default for both C and assembler

 arch/arm/Makefile               |   13 +++++++------
 arch/arm/boot/Makefile          |   10 +++++-----
 arch/arm/common/it8152.c        |   12 +++++++++---
 arch/arm/include/asm/flat.h     |    2 +-
 arch/arm/include/asm/uaccess.h  |    4 ++--
 arch/arm/tools/Makefile         |    2 +-
 drivers/clk/clkdev.c            |    2 +-
 drivers/pcmcia/pxa2xx_sharpsl.c |    2 +-
 drivers/scsi/arm/fas216.c       |    2 +-
 drivers/scsi/arm/oak.c          |    1 +
 drivers/spi/spi-s3c64xx.c       |    2 +-
 drivers/usb/host/ehci-orion.c   |    2 +-
 drivers/video/nuc900fb.c        |    2 +-
 fs/namei.c                      |    6 +++---
 include/linux/cgroup.h          |    2 +-
 mm/slob.c                       |    6 +++---
 net/netfilter/ipvs/ip_vs_ctl.c  |   10 ++++++----
 17 files changed, 45 insertions(+), 35 deletions(-)

Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Ben Blum <bblum@andrew.cmu.edu>
Cc: Ben Dooks <ben-linux@fluff.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Bryan Wu <bryan.wu@canonical.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: Dave Martin <dave.martin@linaro.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: Dominik Brodowski <linux@dominikbrodowski.net>
Cc: Eric Miao <eric.y.miao@gmail.com>
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Greg Ungerer <gerg@uclinux.org>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Cc: Igor Grinberg <grinberg@compulab.co.il>
Cc: Jan Kara <jack@suse.cz>
Cc: Jochen Friedrich <jochen@scram.de>
Cc: Julian Anastasov <ja@ssi.bg>
Cc: Krzysztof Halasa <khc@pm.waw.pl>
Cc: Kukjin Kim <kgene.kim@samsung.com>
Cc: Li Zefan <lizefan@huawei.com>
Cc: Michal Marek <mmarek@suse.cz>
Cc: Mike Rapoport <mike@compulab.co.il>
Cc: Mike Turquette <mturquette@linaro.org>
Cc: Nicolas Pitre <nicolas.pitre@linaro.org>
Cc: Pavel Machek <pavel@suse.cz>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Simon Horman <horms@verge.net.au>
Cc: Tejun Heo <tj@kernel.org>
Cc: Wan ZongShun <mcuos.com@gmail.com>
Cc: coreteam@netfilter.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-fbdev@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-pcmcia@lists.infradead.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
Cc: linux-usb@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: netfilter-devel@vger.kernel.org
Cc: netfilter@vger.kernel.org
Cc: spi-devel-general@lists.sourceforge.net


-- 
1.7.10

[PATCH 05/16] vfs: bogus warnings in fs/namei.c

[Arnd Bergmann]

The follow_link() function always initializes its *p argument,
or returns an error, but not all versions of gcc figure this
out, so we have to work around this using the uninitialized_var()
macro.

Without this patch, building with arm-linux-gnueabi-gcc-4.6 results in:

fs/namei.c: In function 'link_path_walk':
fs/namei.c:649:24: warning: 'cookie' may be used uninitialized in this function [-Wuninitialized]
fs/namei.c:1544:9: note: 'cookie' was declared here
fs/namei.c: In function 'path_lookupat':
fs/namei.c:649:24: warning: 'cookie' may be used uninitialized in this function [-Wuninitialized]
fs/namei.c:1934:10: note: 'cookie' was declared here
fs/namei.c: In function 'path_openat':
fs/namei.c:649:24: warning: 'cookie' may be used uninitialized in this function [-Wuninitialized]
fs/namei.c:2899:9: note: 'cookie' was declared here

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org
Cc: Jan Kara <jack@suse.cz>
---
 fs/namei.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index dd1ed1b..62a1725 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1543,7 +1543,7 @@ static inline int nested_symlink(struct path *path, struct nameidata *nd)
 
 	do {
 		struct path link = *path;
-		void *cookie;
+		void *uninitialized_var(cookie);
 
 		res = follow_link(&link, nd, &cookie);
 		if (res)
@@ -1933,7 +1933,7 @@ static int path_lookupat(int dfd, const char *name,
 	if (!err && !(flags & LOOKUP_PARENT)) {
 		err = lookup_last(nd, &path);
 		while (err > 0) {
-			void *cookie;
+			void *uninitialized_var(cookie);
 			struct path link = path;
 			err = may_follow_link(&link, nd);
 			if (unlikely(err))
@@ -2902,7 +2902,7 @@ static struct file *path_openat(int dfd, const char *pathname,
 	error = do_last(nd, &path, file, op, &opened, pathname);
 	while (unlikely(error > 0)) { /* trailing symlink */
 		struct path link = path;
-		void *cookie;
+		void *uninitialized_var(cookie);
 		if (!(nd->flags & LOOKUP_FOLLOW)) {
 			path_put_conditional(&path, nd);
 			path_put(&nd->path);
-- 
1.7.10

Re: [PATCH 05/16] vfs: bogus warnings in fs/namei.c

[Jan Kara]

On Fri 05-10-12 16:55:19, Arnd Bergmann wrote:
> The follow_link() function always initializes its *p argument,
> or returns an error, but not all versions of gcc figure this
> out, so we have to work around this using the uninitialized_var()
> macro.
  Well, I'm somewhat sceptical to this approach. I agree that bogus
warnings are not nice but later when the code is changed and possibly real
use without initialization is added, we won't notice it. Without changing
anything, we'd at least have a chance of catching it with gcc versions
which were clever enough to not warn with the original code. Or
alternatively if we unconditionally initialized the variable that would get
rid of the warning and made the code more future-proof (that's what I
usually end up doing)... I don't really care that much about the chosen
solution, Al is the one to decide. But I wanted to point out there are
downsides to your solution.

								Honza

> 
> Without this patch, building with arm-linux-gnueabi-gcc-4.6 results in:
> 
> fs/namei.c: In function 'link_path_walk':
> fs/namei.c:649:24: warning: 'cookie' may be used uninitialized in this function [-Wuninitialized]
> fs/namei.c:1544:9: note: 'cookie' was declared here
> fs/namei.c: In function 'path_lookupat':
> fs/namei.c:649:24: warning: 'cookie' may be used uninitialized in this function [-Wuninitialized]
> fs/namei.c:1934:10: note: 'cookie' was declared here
> fs/namei.c: In function 'path_openat':
> fs/namei.c:649:24: warning: 'cookie' may be used uninitialized in this function [-Wuninitialized]
> fs/namei.c:2899:9: note: 'cookie' was declared here
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: linux-fsdevel@vger.kernel.org
> Cc: Jan Kara <jack@suse.cz>
> ---
>  fs/namei.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index dd1ed1b..62a1725 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1543,7 +1543,7 @@ static inline int nested_symlink(struct path *path, struct nameidata *nd)
>  
>  	do {
>  		struct path link = *path;
> -		void *cookie;
> +		void *uninitialized_var(cookie);
>  
>  		res = follow_link(&link, nd, &cookie);
>  		if (res)
> @@ -1933,7 +1933,7 @@ static int path_lookupat(int dfd, const char *name,
>  	if (!err && !(flags & LOOKUP_PARENT)) {
>  		err = lookup_last(nd, &path);
>  		while (err > 0) {
> -			void *cookie;
> +			void *uninitialized_var(cookie);
>  			struct path link = path;
>  			err = may_follow_link(&link, nd);
>  			if (unlikely(err))
> @@ -2902,7 +2902,7 @@ static struct file *path_openat(int dfd, const char *pathname,
>  	error = do_last(nd, &path, file, op, &opened, pathname);
>  	while (unlikely(error > 0)) { /* trailing symlink */
>  		struct path link = path;
> -		void *cookie;
> +		void *uninitialized_var(cookie);
>  		if (!(nd->flags & LOOKUP_FOLLOW)) {
>  			path_put_conditional(&path, nd);
>  			path_put(&nd->path);
> -- 
> 1.7.10
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH 05/16] vfs: bogus warnings in fs/namei.c

[Arnd Bergmann]

On Monday 08 October 2012, Jan Kara wrote:
> On Fri 05-10-12 16:55:19, Arnd Bergmann wrote:
> > The follow_link() function always initializes its *p argument,
> > or returns an error, but not all versions of gcc figure this
> > out, so we have to work around this using the uninitialized_var()
> > macro.
>   Well, I'm somewhat sceptical to this approach. I agree that bogus
> warnings are not nice but later when the code is changed and possibly real
> use without initialization is added, we won't notice it. Without changing
> anything, we'd at least have a chance of catching it with gcc versions
> which were clever enough to not warn with the original code. Or
> alternatively if we unconditionally initialized the variable that would get
> rid of the warning and made the code more future-proof (that's what I
> usually end up doing)... I don't really care that much about the chosen
> solution, Al is the one to decide. But I wanted to point out there are
> downsides to your solution.

I'll drop the patch for now and won't send it from my tree then. I agree
that uninitialized_var() is not ideal, but none of the alternatives seemed
better.

With my latest compiler, I don't actually see the warnings any more, so
maybe someone fixed gcc instead, or this went away after another change.
I'll let you know if it comes back so we can discuss about a better fix then.

	Arnd

Re: [PATCH 05/16] vfs: bogus warnings in fs/namei.c

[Arnd Bergmann]

On Tuesday 09 October 2012, Arnd Bergmann wrote:
> On Monday 08 October 2012, Jan Kara wrote:
> > On Fri 05-10-12 16:55:19, Arnd Bergmann wrote:
> > > The follow_link() function always initializes its *p argument,
> > > or returns an error, but not all versions of gcc figure this
> > > out, so we have to work around this using the uninitialized_var()
> > > macro.
> >   Well, I'm somewhat sceptical to this approach. I agree that bogus
> > warnings are not nice but later when the code is changed and possibly real
> > use without initialization is added, we won't notice it. Without changing
> > anything, we'd at least have a chance of catching it with gcc versions
> > which were clever enough to not warn with the original code. Or
> > alternatively if we unconditionally initialized the variable that would get
> > rid of the warning and made the code more future-proof (that's what I
> > usually end up doing)... I don't really care that much about the chosen
> > solution, Al is the one to decide. But I wanted to point out there are
> > downsides to your solution.
> 
> I'll drop the patch for now and won't send it from my tree then. I agree
> that uninitialized_var() is not ideal, but none of the alternatives seemed
> better.
> 
> With my latest compiler, I don't actually see the warnings any more, so
> maybe someone fixed gcc instead, or this went away after another change.
> I'll let you know if it comes back so we can discuss about a better fix then.
> 

Update: I could actually reproduce the problem now, but it only happens when
building with 'gcc -s' (i.e. CONFIG_CC_OPTIMIZE_FOR_SIZE). It does happen
with both gcc-4.6 and with gcc-4.8, and on both x86-64 and ARM. An alternative
patch that would also make it go away is the variant below, but I think that's
even worse than the first version I suggested because it makes the binary
output slightly worse by adding an unnecessary initialization when building with
'make -s'.

	Arnd

diff --git a/fs/namei.c b/fs/namei.c
index aa30d19..c3612a5 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -810,6 +810,7 @@ follow_link(struct path *link, struct nameidata *nd, void **p)
 	return error;
 
 out_put_nd_path:
+	*p = NULL;
 	path_put(&nd->path);
 	path_put(link);
 	return error;

Re: [PATCH 05/16] vfs: bogus warnings in fs/namei.c

[Jan Kara]

On Tue 09-10-12 13:07:19, Arnd Bergmann wrote:
> On Tuesday 09 October 2012, Arnd Bergmann wrote:
> > On Monday 08 October 2012, Jan Kara wrote:
> > > On Fri 05-10-12 16:55:19, Arnd Bergmann wrote:
> > > > The follow_link() function always initializes its *p argument,
> > > > or returns an error, but not all versions of gcc figure this
> > > > out, so we have to work around this using the uninitialized_var()
> > > > macro.
> > >   Well, I'm somewhat sceptical to this approach. I agree that bogus
> > > warnings are not nice but later when the code is changed and possibly real
> > > use without initialization is added, we won't notice it. Without changing
> > > anything, we'd at least have a chance of catching it with gcc versions
> > > which were clever enough to not warn with the original code. Or
> > > alternatively if we unconditionally initialized the variable that would get
> > > rid of the warning and made the code more future-proof (that's what I
> > > usually end up doing)... I don't really care that much about the chosen
> > > solution, Al is the one to decide. But I wanted to point out there are
> > > downsides to your solution.
> > 
> > I'll drop the patch for now and won't send it from my tree then. I agree
> > that uninitialized_var() is not ideal, but none of the alternatives seemed
> > better.
> > 
> > With my latest compiler, I don't actually see the warnings any more, so
> > maybe someone fixed gcc instead, or this went away after another change.
> > I'll let you know if it comes back so we can discuss about a better fix then.
> > 
> 
> Update: I could actually reproduce the problem now, but it only happens when
> building with 'gcc -s' (i.e. CONFIG_CC_OPTIMIZE_FOR_SIZE). It does happen
> with both gcc-4.6 and with gcc-4.8, and on both x86-64 and ARM. An alternative
> patch that would also make it go away is the variant below, but I think that's
> even worse than the first version I suggested because it makes the binary
> output slightly worse by adding an unnecessary initialization when building with
> 'make -s'.
  Hum, dumb compiler... I like this patch better and since the extra
initialization is on error path only, I don't think it matters. But
whatever Al likes better.

									Honza

> diff --git a/fs/namei.c b/fs/namei.c
> index aa30d19..c3612a5 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -810,6 +810,7 @@ follow_link(struct path *link, struct nameidata *nd, void **p)
>  	return error;
>  
>  out_put_nd_path:
> +	*p = NULL;
>  	path_put(&nd->path);
>  	path_put(link);
>  	return error;

Re: [PATCH 05/16] vfs: bogus warnings in fs/namei.c

[Al Viro]

On Tue, Oct 09, 2012 at 01:07:19PM +0000, Arnd Bergmann wrote:

> Update: I could actually reproduce the problem now, but it only happens when
> building with 'gcc -s' (i.e. CONFIG_CC_OPTIMIZE_FOR_SIZE). It does happen
> with both gcc-4.6 and with gcc-4.8, and on both x86-64 and ARM. An alternative
> patch that would also make it go away is the variant below, but I think that's
> even worse than the first version I suggested because it makes the binary
> output slightly worse by adding an unnecessary initialization when building with
> 'make -s'.

I can live with that, provided that you give it sane commit message and
your s-o-b.

[PATCH v2] vfs: bogus warnings in fs/namei.c

[Arnd Bergmann]

The follow_link() function always initializes its *p argument,
or returns an error, but when building with 'gcc -s', the compiler
gets confused by the __always_inline attribute to the function
and can no longer detect where the cookie was initialized.

The solution is to always initialize the pointer from follow_link,
even in the error path. When building with -O2, this has zero impact
on generated code and adds a single instruction in the error path
for a -Os build on ARM.

Without this patch, building with gcc-4.6 through gcc-4.8 and
CONFIG_CC_OPTIMIZE_FOR_SIZE results in:

fs/namei.c: In function 'link_path_walk':
fs/namei.c:649:24: warning: 'cookie' may be used uninitialized in this function [-Wuninitialized]
fs/namei.c:1544:9: note: 'cookie' was declared here
fs/namei.c: In function 'path_lookupat':
fs/namei.c:649:24: warning: 'cookie' may be used uninitialized in this function [-Wuninitialized]
fs/namei.c:1934:10: note: 'cookie' was declared here
fs/namei.c: In function 'path_openat':
fs/namei.c:649:24: warning: 'cookie' may be used uninitialized in this function [-Wuninitialized]
fs/namei.c:2899:9: note: 'cookie' was declared here

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/fs/namei.c b/fs/namei.c
index 6d47fac..c1f18e4 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -810,6 +810,7 @@ follow_link(struct path *link, struct nameidata *nd, void **p)
 	return error;
 
 out_put_nd_path:
+	*p = NULL;
 	path_put(&nd->path);
 	path_put(link);
 	return error;