From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1JNc8r-0005O7-Bi for mharc-grub-devel@gnu.org; Fri, 08 Feb 2008 17:56:21 -0500 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1JNc8p-0005Mz-PZ for grub-devel@gnu.org; Fri, 08 Feb 2008 17:56:19 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1JNc8n-0005LN-Lj for grub-devel@gnu.org; Fri, 08 Feb 2008 17:56:19 -0500 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JNc8n-0005L8-El for grub-devel@gnu.org; Fri, 08 Feb 2008 17:56:17 -0500 Received: from neonescio.viaisn.org ([82.94.249.43]) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1JNc8n-0007kE-9g for grub-devel@gnu.org; Fri, 08 Feb 2008 17:56:17 -0500 Received: from dijkstra.dekkers.cx ([2001:960:7a0:0:213:d4ff:fe9c:2487] ident=Debian-exim) by neonescio.viaisn.org with esmtpsa (TLS-1.0:RSA_AES_256_CBC_SHA1:32 CV=0) (Exim 4.63 #1) id 1JNc8e-00044V-6O for grub-devel@gnu.org; Fri, 08 Feb 2008 23:56:08 +0100 Received: from localhost ([127.0.0.1] helo=dijkstra.dekkers.cx ident=jeroen) by dijkstra.dekkers.cx with esmtp (Exim 4.68) (envelope-from ) id 1JNc8d-0002Gy-Fu for grub-devel@gnu.org; Fri, 08 Feb 2008 23:56:07 +0100 Date: Fri, 08 Feb 2008 23:56:07 +0100 Message-ID: <874pcj9hx4.wl@dekkers.cx> From: Jeroen Dekkers To: The development of GRUB 2 In-Reply-To: <20080206225954.GA17090@thorin> References: <20080206164507.GA3287@thorin> <87bq6thfj8.wl@dekkers.cx> <20080206225954.GA17090@thorin> User-Agent: Wanderlust/2.14.0 (Africa) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?ISO-8859-4?Q?Goj=F2?=) APEL/10.7 Emacs/22.1 (x86_64-pc-linux-gnu) MULE/5.0 (SAKAKI) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII X-detected-kernel: by monty-python.gnu.org: Linux 2.6 (newer, 3) Subject: Re: [PATCH] simplify grub_raid_array X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GRUB 2 List-Id: The development of GRUB 2 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Feb 2008 22:56:20 -0000 At Wed, 6 Feb 2008 23:59:54 +0100, Robert Millan wrote: > > On Wed, Feb 06, 2008 at 11:43:39PM +0100, Jeroen Dekkers wrote: > > At Wed, 6 Feb 2008 17:45:07 +0100, > > Robert Millan wrote: > > > Unless I missed something, it seems that grub_raid_array contains redundant > > > information (`name' is already present via `disk->name'). I propose to > > > simplify it this way. > > > > No idea why, I don't have the time to look at the actual code, but > > > > > @@ -410,7 +410,7 @@ grub_raid_scan_device (const char *name) > > > return 0; > > > } > > > > > > - if (array->device[sb.this_disk.number].name != 0) > > > + if (array->device[sb.this_disk.number]->name != 0) > > > { > > > /* We found multiple devices with the same number. Again, > > > this shouldn't happen.*/ > > > > looks suspicious to me. Is that really doing what it is meant to do? > > Yes. No, it doesn't. The meaning of the check .name != 0 is whether a device with that number already exists in the array. If .name is 0, it doesn't exist yet, because else it would've been assigned something. What you're now doing is wrong because if there isn't a device in the array then array->device[sb.this_disk.number] is 0. I guess the only reason this actually works is because the first page of memory (the one at address 0) happens to be zero by luck. Jeroen Dekkers