unitialized variable in 2.4.7 (sym53c8xx, dmi_scan)

unitialized variable in 2.4.7 (sym53c8xx, dmi_scan)

[richard offer]

Compiling with -Werror to catch my development mistakes gives.

sym53c8xx.c:6994: warning: `istat' might be used uninitialized in this
function

dmi_scan.c:161: warning: `disable_ide_dma' defined but not used

The following fixes these issues.

8<--------------------------------------------------------------
===== arch/i386/kernel/dmi_scan.c 1.3 vs edited =====
--- 1.3/arch/i386/kernel/dmi_scan.c     Wed Jul 18 02:43:27 2001
+++ edited/arch/i386/kernel/dmi_scan.c  Wed Jul 25 13:41:59 2001
@@ -157,6 +157,7 @@
  *     corruption problems
  */ 
  
+#if 0 /* commented out until its used in dmi_blacklist[] */
 static __init int disable_ide_dma(struct dmi_blacklist *d)
 {
 #ifdef CONFIG_BLK_DEV_IDE
@@ -169,6 +170,7 @@
 #endif
        return 0;
 }
+#endif
 
 /* 
  * Some machines require the "reboot=b"  commandline option, this quirk
makes that automatic.
===== drivers/scsi/sym53c8xx.c 1.6 vs edited =====
--- 1.6/drivers/scsi/sym53c8xx.c        Thu Jul  5 04:28:16 2001
+++ edited/drivers/scsi/sym53c8xx.c     Wed Jul 25 13:37:10 2001
@@ -6991,7 +6991,7 @@
 
 static void ncr_soft_reset(ncb_p np)
 {
-       u_char istat;
+       u_char istat=0;
        int i;
 
        if (!(np->features & FE_ISTAT1) || !(INB (nc_istat1) & SRUN))
8<--------------------------------------------------------------

Re: unitialized variable in 2.4.7 (sym53c8xx, dmi_scan)

[Alan Cox]

>  static __init int disable_ide_dma(struct dmi_blacklist *d)
>  {
>  #ifdef CONFIG_BLK_DEV_IDE
> @@ -169,6 +170,7 @@
>  #endif
>         return 0;
>  }
> +#endif

This just makes it harder to finish the merges

> makes that automatic.
> ===== drivers/scsi/sym53c8xx.c 1.6 vs edited =====
> --- 1.6/drivers/scsi/sym53c8xx.c        Thu Jul  5 04:28:16 2001
> +++ edited/drivers/scsi/sym53c8xx.c     Wed Jul 25 13:37:10 2001
> @@ -6991,7 +6991,7 @@
>  
>  static void ncr_soft_reset(ncb_p np)
>  {
> -       u_char istat;
> +       u_char istat=0;
>         int i;
>  
>         if (!(np->features & FE_ISTAT1) || !(INB (nc_istat1) & SRUN))

And this means when we get a real bug with istat not being assigned it
wont be seen.

Re: unitialized variable in 2.4.7 (sym53c8xx, dmi_scan)

[richard offer]

* frm alan@lxorguk.ukuu.org.uk "07/25/01 22:25:32 +0100" | sed '1,$s/^/* /'
*
*>  static __init int disable_ide_dma(struct dmi_blacklist *d)
*>  {
*>  # ifdef CONFIG_BLK_DEV_IDE
*> @@ -169,6 +170,7 @@
*>  # endif
*>         return 0;
*>  }
*> +#endif
* 
* This just makes it harder to finish the merges

Huh ? Was it a bad patch ?

* 
*> makes that automatic.
*> ===== drivers/scsi/sym53c8xx.c 1.6 vs edited =====
*> --- 1.6/drivers/scsi/sym53c8xx.c        Thu Jul  5 04:28:16 2001
*> +++ edited/drivers/scsi/sym53c8xx.c     Wed Jul 25 13:37:10 2001
*> @@ -6991,7 +6991,7 @@
*>  
*>  static void ncr_soft_reset(ncb_p np)
*>  {
*> -       u_char istat;
*> +       u_char istat=0;
*>         int i;
*>  
*>         if (!(np->features & FE_ISTAT1) || !(INB (nc_istat1) & SRUN))
* 
* And this means when we get a real bug with istat not being assigned it
* wont be seen.
* 

Isn't the only way that istat could be unassigned would be for the for loop
never to be executed. An unlikely event, but the compiler sees it as a
warning and means its not possible to build the code with -Werror.


richard.

-----------------------------------------------------------------------
Richard Offer                     Technical Lead, Trust Technology, SGI
"Specialization is for insects"
_______________________________________________________________________

Re: unitialized variable in 2.4.7 (sym53c8xx, dmi_scan)

[Gérard Roudier]

On Wed, 25 Jul 2001, Alan Cox wrote:

> >  static __init int disable_ide_dma(struct dmi_blacklist *d)
> >  {
> >  #ifdef CONFIG_BLK_DEV_IDE
> > @@ -169,6 +170,7 @@
> >  #endif
> >         return 0;
> >  }
> > +#endif
>
> This just makes it harder to finish the merges
>
> > makes that automatic.
> > ===== drivers/scsi/sym53c8xx.c 1.6 vs edited =====
> > --- 1.6/drivers/scsi/sym53c8xx.c        Thu Jul  5 04:28:16 2001
> > +++ edited/drivers/scsi/sym53c8xx.c     Wed Jul 25 13:37:10 2001
> > @@ -6991,7 +6991,7 @@
> >
> >  static void ncr_soft_reset(ncb_p np)
> >  {
> > -       u_char istat;
> > +       u_char istat=0;
> >         int i;
> >
> >         if (!(np->features & FE_ISTAT1) || !(INB (nc_istat1) & SRUN))
>
> And this means when we get a real bug with istat not being assigned it
> wont be seen.

This will not happen (istat will never be used unitialised).
The compiler has enough information to know about this at compile time as
the full code below demonstrates clearly:

--

static void ncr_soft_reset(ncb_p np)
{
	u_char istat;
	int i;

	if (!(np->features & FE_ISTAT1) || !(INB (nc_istat1) & SRUN))
		goto do_chip_reset;

	OUTB (nc_istat, CABRT);
	for (i = 100000 ; i ; --i) {
		istat = INB (nc_istat);
		if (istat & SIP) {
			INW (nc_sist);
		}
		else if (istat & DIP) {
			if (INB (nc_dstat) & ABRT);
				break;
		}
		UDELAY(5);
	}
	OUTB (nc_istat, 0);
	if (!i)
		printk("%s: unable to abort current chip operation, "
		       "ISTAT=0x%02x.\n", ncr_name(np), istat);
do_chip_reset:
	ncr_chip_reset(np);
}

--

IMO, the problem should be reported to the compiler maintainers. By
assigning some fake definitions to FE_ISTAT1, SRUN, INB(), etc...,
the code above should help them fix the compiler paranoia disease.
Changing the driver code looks like an odd idea.

  Gérard.