From owner-freebsd-bugs  Tue May 27 15:29:27 1997
Return-Path: <owner-bugs>
Received: (from root@localhost)
          by hub.freebsd.org (8.8.5/8.8.5) id PAA07438
          for bugs-outgoing; Tue, 27 May 1997 15:29:27 -0700 (PDT)
Received: from helbig.informatik.ba-stuttgart.de (helbig.informatik.ba-stuttgart.de [141.31.166.22])
          by hub.freebsd.org (8.8.5/8.8.5) with ESMTP id PAA07427
          for <freebsd-bugs@hub.freebsd.org>; Tue, 27 May 1997 15:29:23 -0700 (PDT)
Received: (from helbig@localhost)
	by helbig.informatik.ba-stuttgart.de (8.8.5/8.8.5) id AAA20430
	for freebsd-bugs@hub.freebsd.org; Wed, 28 May 1997 00:29:04 +0200 (MET DST)
From: Wolfgang Helbig <helbig@MX.BA-Stuttgart.De>
Message-Id: <199705272229.AAA20430@helbig.informatik.ba-stuttgart.de>
Subject: Re: kern/3696: kernel panic during wd hard disk probe if CMD640 is enabled
In-Reply-To: <199705271210.FAA10522@hub.freebsd.org> from Stefan Esser at "May 27, 97 05:10:01 am"
To: freebsd-bugs@hub.freebsd.org
Date: Wed, 28 May 1997 00:29:03 +0200 (MET DST)
X-Mailer: ELM [version 2.4ME+ PL30 (25)]
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: owner-bugs@FreeBSD.ORG
X-Loop: FreeBSD.org
Precedence: bulk

> The following reply was made to PR kern/3696; it has been noted by GNATS.
> 
> From: Stefan Esser <se@FreeBSD.ORG>
> To: Joel.Faedi@esial.u-nancy.fr
> Cc: freebsd-gnats-submit@freebsd.org
> Subject: Re: kern/3696: kernel panic during wd hard disk probe if CMD640 is enabled
> Date: Tue, 27 May 1997 14:05:46 +0200
> 
>  On May 27, Joel.Faedi@esial.u-nancy.fr wrote:
>  > >Description:
>  > kernel panics while testing wd hard drive if "options CMD640" is activated and if you don't have wd1 connected (even if the controleris a CMD640 or not).
>  > >How-To-Repeat:
>  > compile a kernel with "options CMD640", put a single hard disk onyour first IDE controler, system panic while testing hard disks.
>  
>  > >Fix:
>  > in file /usr/src/sys/i386/isa/wd.c, in function "wdattach", change the lines:-------------------------------------------------
>  > #ifdef CMD640
>  >      wdtab[du->dk_ctrlr_cmd640].b_active = 2;
>  > #else
>  >       wdtab[dvp->id_unit].b_active = 2;
>  > #endif-------------------------------------------------
>  
>  > by the lines:-------------------------------------------------
>  > #ifdef CMD640
>  >       if (eide_quirks & Q_CMD640B) {
>  >         wdtab[PRIMARY].b_active = 2;
>  >       } else {
>  >         wdtab[dvp->id_unit].b_active = 2;
>  >       }
>  > #else
>  >       wdtab[dvp->id_unit].b_active = 2;
>  > #endif
>  
>  Sorry, but I don't see what this patch changes.
>  
>  du->dk_ctrlr_cmd640 is set to either PRIMARY or du->dk_ctrlr
>  (which is identical to dvp->id_unit) early in wdattach, and
>  the value choosen is made dependent on the same condition as
>  you suggest above.
>  
>  Are you sure the patch makes a difference ?
>  I can't see how it could ...
>  
>  Regards, STefan

The patch does make a difference if du == NULL. This might happen
if a nonexisting drive is configured, e. g. in the GENERIC
configuration file.  Then the loop iterating all configured wd-drives
in isa_biotab_wdc might be left with a freed du. This did not occur
on my system with a GENERIC kernel, options "CMD640" enabled and
one drive attached to the second channel -- and this is what I do
not understand!

Anyway, depending on du pointing to a valid disk structure outside
the loop is *very* bad style and I apologize for this oversight.
I am very glad, the options "CMD640" were not enabled by default
though.

Thanks to Joel for the patch and to Stefan for committing it!

Wolfgang