From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1KaXLu-0005TD-9d for mharc-grub-devel@gnu.org; Tue, 02 Sep 2008 10:59:30 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KaXLs-0005SX-I5 for grub-devel@gnu.org; Tue, 02 Sep 2008 10:59:28 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KaXLq-0005Rw-RP for grub-devel@gnu.org; Tue, 02 Sep 2008 10:59:28 -0400 Received: from [199.232.76.173] (port=59114 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KaXLq-0005Rt-OT for grub-devel@gnu.org; Tue, 02 Sep 2008 10:59:26 -0400 Received: from mta-out.inet.fi ([195.156.147.13]:39694 helo=jenni1.inet.fi) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KaXLq-0003i5-HB for grub-devel@gnu.org; Tue, 02 Sep 2008 10:59:26 -0400 Received: from [127.0.0.1] (88.193.32.97) by jenni1.inet.fi (8.5.014) id 488F15340197D613 for grub-devel@gnu.org; Tue, 2 Sep 2008 17:59:24 +0300 Message-ID: <48BD54CD.3000907@nic.fi> Date: Tue, 02 Sep 2008 17:59:25 +0300 From: =?ISO-8859-1?Q?Vesa_J=E4=E4skel=E4inen?= User-Agent: Thunderbird 2.0.0.16 (Windows/20080708) MIME-Version: 1.0 To: The development of GRUB 2 References: <20080808202337.3440.57502.reportbug@sylvester.jejik.com.jejik.com> <1218277583.9301.6.camel@fz.local> <489EEAC5.4080209@jejik.com> <1218471512.6939.19.camel@fz.local> <1218735245.8950.10.camel@fz.local> <20080814180353.GC5614@thorin> <1218742324.10393.8.camel@fz.local> <20080814203856.GA23307@thorin> <1218748505.10393.13.camel@fz.local> <1219071926.4569.57.camel@fz.local> <20080901221405.GA11511@thorin> <1220309171.4115.31.camel@fz.local> <1220344081.4101.2.camel@fz.local> In-Reply-To: <1220344081.4101.2.camel@fz.local> X-Enigmail-Version: 0.95.7 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-detected-kernel: by monty-python.gnu.org: Linux 2.6 (newer, 3) Subject: Re: [PATCH] Grub2 cannot find LVM volume groups with a dash (-) in the name 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: Tue, 02 Sep 2008 14:59:28 -0000 Felix Zielcke wrote: > Am Dienstag, den 02.09.2008, 00:46 +0200 schrieb Felix Zielcke: > >> I'm too lazy now to make a new patch and go sleeping now. >> [...] >> >> I hope that Marco could have a quick look over it especially the >> changelog part :) > > Final patch attached. > In changelog I had again past and present mixed and I just use now the > `normal' types instead of grub_uintN_t, seems like that's more used on > util/ > So if there are no objections I'd like to commit this :) > + unsigned short i, j; > + const unsigned char k = sizeof ("/dev/mapper/") - 1; > + const unsigned short l = strlen (os_dev) - k + 1; sizeof returns type of size_t so it would be good that char k uses that. I am a bit surprised that this didn't generate compiler warning? And there is no reason to define integers as constants unless you really want to make sure they don't change :) I assume we have grub_size_t or comparable there. > + const unsigned short l = strlen (os_dev) - k + 1; > > - break; > + grub_dev = xmalloc (strlen (os_dev) - k + 1); if you already calculate something to variable it would be nice to re-use that calculation again. > + for (i = 0, j = 0; i < l; i++, j++) > + { > + grub_dev[j] = os_dev[k + i]; > + if (os_dev[k + i] == '-' && os_dev[k + i + 1] == '-') > + i++; > + } Can't you index i: k <= i < strlen(os_dev)? Would make this a bit more readable. Or as you could just increment k in every loop and use that to index. Your 0 <= j < l should limit char array. You could use a bit more descriptive names like l->len, k->start/offset. For indexes we quite often use i,j,k. > + p = strchr (os_dev, 'p'); > + if (p) > + *p = ','; It is usually a bad idea to modify source string.