[PATCH] 2.4.19 warnings cleanup

[PATCH] 2.4.19 warnings cleanup

[Krzysztof Halasa]

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

Hi,

The attached patch eliminates some compiler warning messages from
various parts of the kernel. It disables some unused code (compressed
ramdisk routines if no ramdisk support). Shouldn't break anything,
but I wouldn't rather apply it in final rc-* stages...

2.4.19 only.
-- 
Krzysztof Halasa
Network Administrator

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: warnings.patch --]
[-- Type: text/x-patch, Size: 2848 bytes --]

--- linux/drivers/char/agp/agpgart_be.c.orig	Sat Aug  3 18:30:26 2002
+++ linux/drivers/char/agp/agpgart_be.c	Sat Aug  3 18:32:41 2002
@@ -397,7 +397,7 @@
 static void agp_generic_agp_enable(u32 mode)
 {
 	struct pci_dev *device = NULL;
-	u32 command, scratch, cap_id;
+	u32 command, scratch;
 	u8 cap_ptr;
 
 	pci_read_config_dword(agp_bridge.dev,
@@ -4295,7 +4295,6 @@
 {
 	struct pci_dev *dev = NULL;
 	u8 cap_ptr = 0x00;
-	u32 cap_id, scratch;
 
 	if ((dev = pci_find_class(PCI_CLASS_BRIDGE_HOST << 8, NULL)) == NULL)
 		return -ENODEV;
--- linux/arch/i386/kernel/dmi_scan.c.orig	Sat Aug  3 17:13:28 2002
+++ linux/arch/i386/kernel/dmi_scan.c	Sat Aug  3 19:02:47 2002
@@ -186,12 +186,13 @@
 #define MATCH(a,b)	{ a, b }
 
 /*
- *	We have problems with IDE DMA on some platforms. In paticular the
+ *	We have problems with IDE DMA on some platforms. In particular the
  *	KT7 series. On these it seems the newer BIOS has fixed them. The
  *	rule needs to be improved to match specific BIOS revisions with
  *	corruption problems
- */ 
- 
+ */
+
+#if 0
 static __init int disable_ide_dma(struct dmi_blacklist *d)
 {
 #ifdef CONFIG_BLK_DEV_IDE
@@ -204,6 +205,7 @@
 #endif	
 	return 0;
 }
+#endif
 
 /* 
  * Reboot options and system auto-detection code provided by
--- linux/fs/dnotify.c.orig	Sat Aug  3 17:14:15 2002
+++ linux/fs/dnotify.c	Sat Aug  3 18:33:53 2002
@@ -135,7 +135,7 @@
 	}
 	if (changed)
 		redo_inode_mask(inode);
-out:
+
 	write_unlock(&dn_lock);
 }
 
--- linux/init/do_mounts.c.orig	Sat Aug  3 17:14:32 2002
+++ linux/init/do_mounts.c	Sat Aug  3 18:26:32 2002
@@ -882,6 +882,7 @@
 #define WSIZE 0x8000    /* window size--must be a power of two, and */
 			/*  at least 32K for zip's deflate method */
 
+#ifdef CONFIG_BLK_DEV_RAM
 static uch *inbuf;
 static uch *window;
 
@@ -1006,5 +1007,6 @@
 	kfree(window);
 	return result;
 }
+#endif /* CONFIG_BLK_DEV_RAM */
 
 #endif  /* BUILD_CRAMDISK */
--- linux/include/linux/namespace.h.orig	Sat Aug  3 17:14:31 2002
+++ linux/include/linux/namespace.h	Sat Aug  3 18:46:43 2002
@@ -38,5 +38,6 @@
 	atomic_inc(&namespace->count);
 }
 
+int __init init_rootfs(void);
 #endif
 #endif
--- linux/drivers/net/ppp_generic.c.orig	Sat Aug  3 17:13:58 2002
+++ linux/drivers/net/ppp_generic.c	Sat Aug  3 19:11:54 2002
@@ -378,7 +378,7 @@
 {
 	struct ppp_file *pf = file->private_data;
 	DECLARE_WAITQUEUE(wait, current);
-	ssize_t ret;
+	ssize_t ret = 0; /* suppress compiler warning */
 	struct sk_buff *skb = 0;
 
 	if (pf == 0)
--- linux/arch/i386/kernel/setup.c.orig	Sat Aug  3 17:13:29 2002
+++ linux/arch/i386/kernel/setup.c	Sat Aug  3 19:17:22 2002
@@ -175,6 +175,8 @@
 static int disable_x86_fxsr __initdata = 0;
 static int disable_x86_ht __initdata = 0;
 
+static int __init have_cpuid_p(void);
+
 int enable_acpi_smp_table;
 
 #if defined(CONFIG_AGP) || defined(CONFIG_AGP_MODULE)

Re: [PATCH] 2.4.19 warnings cleanup

[Alan Cox]

> --- linux/drivers/net/ppp_generic.c.orig	Sat Aug  3 17:13:58 2002
> +++ linux/drivers/net/ppp_generic.c	Sat Aug  3 19:11:54 2002
> @@ -378,7 +378,7 @@
>  {
>  	struct ppp_file *pf = file->private_data;
>  	DECLARE_WAITQUEUE(wait, current);
> -	ssize_t ret;
> +	ssize_t ret = 0; /* suppress compiler warning */
>  	struct sk_buff *skb = 0;
>  
>  	if (pf == 0)


Please don't do this. I'm regularly having to fix drivers where people
hid bugs this way rather than working out if it was a real problem. If
it is genuinely a compiler corner case then let the gcc folks know and
comment it but leave the warning.

Re: [PATCH] 2.4.19 warnings cleanup

[Krzysztof Halasa]

Alan Cox <alan@lxorguk.ukuu.org.uk> writes:

> > --- linux/drivers/net/ppp_generic.c.orig	Sat Aug  3 17:13:58 2002
> > +++ linux/drivers/net/ppp_generic.c	Sat Aug  3 19:11:54 2002
> > @@ -378,7 +378,7 @@
> >  {
> >  	struct ppp_file *pf = file->private_data;
> >  	DECLARE_WAITQUEUE(wait, current);
> > -	ssize_t ret;
> > +	ssize_t ret = 0; /* suppress compiler warning */
> >  	struct sk_buff *skb = 0;
> >  
> >  	if (pf == 0)
> 
> 
> Please don't do this. I'm regularly having to fix drivers where people
> hid bugs this way rather than working out if it was a real problem. If
> it is genuinely a compiler corner case then let the gcc folks know and
> comment it but leave the warning.

Ok.
-- 
Krzysztof Halasa
Network Administrator

Re: [PATCH] 2.4.19 warnings cleanup

[Paul Mackerras]

Alan Cox writes:

> > --- linux/drivers/net/ppp_generic.c.orig	Sat Aug  3 17:13:58 2002
> > +++ linux/drivers/net/ppp_generic.c	Sat Aug  3 19:11:54 2002
> > @@ -378,7 +378,7 @@
> >  {
> >  	struct ppp_file *pf = file->private_data;
> >  	DECLARE_WAITQUEUE(wait, current);
> > -	ssize_t ret;
> > +	ssize_t ret = 0; /* suppress compiler warning */
> >  	struct sk_buff *skb = 0;
> >  
> >  	if (pf == 0)
> 
> 
> Please don't do this. I'm regularly having to fix drivers where people
> hid bugs this way rather than working out if it was a real problem. If
> it is genuinely a compiler corner case then let the gcc folks know and
> comment it but leave the warning.

The code is in ppp_read actually OK; if you trace through the logic
you can prove that ret is never actually used without being set first.

Paul.

Re: [PATCH] 2.4.19 warnings cleanup

[Krzysztof Halasa]

Paul Mackerras <paulus@au1.ibm.com> writes:

> > > --- linux/drivers/net/ppp_generic.c.orig	Sat Aug  3 17:13:58 2002
> > > +++ linux/drivers/net/ppp_generic.c	Sat Aug  3 19:11:54 2002
> > > @@ -378,7 +378,7 @@
> > >  {
> > >  	struct ppp_file *pf = file->private_data;
> > >  	DECLARE_WAITQUEUE(wait, current);
> > > -	ssize_t ret;
> > > +	ssize_t ret = 0; /* suppress compiler warning */
> > >  	struct sk_buff *skb = 0;
> > >  
> The code is in ppp_read actually OK; if you trace through the logic
> you can prove that ret is never actually used without being set first.

That's right - that's exactly why I wrote "suppress compiler warning".
It's just the compiler not smart enough (of course, i looked at the code
paths and I'd just fix it if it was broken).

Anyway, it seems there are more places like that in the kernel tree, so,
as Alan said, the correct thing to improve is the compiler (not even sure
if gcc3 would print the warning, I'm using RH "2.96" gcc).
-- 
Krzysztof Halasa
Network Administrator

Re: [PATCH] 2.4.19 warnings cleanup

[David S. Miller]

   > --- linux/drivers/net/ppp_generic.c.orig	Sat Aug  3 17:13:58 2002
   > +++ linux/drivers/net/ppp_generic.c	Sat Aug  3 19:11:54 2002
   > @@ -378,7 +378,7 @@
   >  {
   >  	struct ppp_file *pf = file->private_data;
   >  	DECLARE_WAITQUEUE(wait, current);
   > -	ssize_t ret;
   > +	ssize_t ret = 0; /* suppress compiler warning */

   Please don't do this. I'm regularly having to fix drivers where people
   hid bugs this way rather than working out if it was a real problem. If
   it is genuinely a compiler corner case then let the gcc folks know and
   comment it but leave the warning.

A compiler isn't able to work out the control flow which
makes sure ret is indeed initialized on every path to
a use.  Solving such a problem is traveling salesman'ish :-)

	for (;;) {
		...
		if (skb)
			break;
		...
		set 'ret' to something
		more break statements
	}

	if (skb == 0)
		goto out; /* where 'ret' is used' */

	set 'ret' to something

See? :-)

Re: [PATCH] 2.4.19 warnings cleanup

[bill davidsen]

In article <20020804.200246.26945647.davem@redhat.com>,
David S. Miller <davem@redhat.com> wrote:

| A compiler isn't able to work out the control flow which
| makes sure ret is indeed initialized on every path to
| a use.  Solving such a problem is traveling salesman'ish :-)

This is true, but on the other hand I don't see much virtue in taking
the attitude that I'm sure the compiler is wrong to avoid the overhead
of initializing the variable or clarifying the code.

I totally agree that there are cases in which the compiler can't be sure
and the programmer is based on some outside assumptions of input values
or whatever, but I don't mind making the code robust in cases other than
innermost loops where I just can't fix it.

I'd rather set the initial value to NOTSET or NULL or some value which
will clearly show if you are setting what you think you are. I've
occasionally been surprised, particularly when trusting values passed
into a procedure.

Just my take on reliable vs. fast.
-- 
bill davidsen <davidsen@tmr.com>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.