From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1KaIA6-0006Em-2s for mharc-grub-devel@gnu.org; Mon, 01 Sep 2008 18:46:18 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KaIA4-0006EP-DV for grub-devel@gnu.org; Mon, 01 Sep 2008 18:46:16 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KaIA2-0006Cv-PQ for grub-devel@gnu.org; Mon, 01 Sep 2008 18:46:16 -0400 Received: from [199.232.76.173] (port=60250 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KaIA2-0006Ck-If for grub-devel@gnu.org; Mon, 01 Sep 2008 18:46:14 -0400 Received: from moutng.kundenserver.de ([212.227.126.187]:50190) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KaIA1-0000kc-Vm for grub-devel@gnu.org; Mon, 01 Sep 2008 18:46:14 -0400 Received: from [85.180.29.137] (e180029137.adsl.alicedsl.de [85.180.29.137]) by mrelayeu.kundenserver.de (node=mrelayeu2) with ESMTP (Nemesis) id 0MKwtQ-1KaIA02PeG-0005SR; Tue, 02 Sep 2008 00:46:12 +0200 From: Felix Zielcke To: The development of GRUB 2 In-Reply-To: <20080901221405.GA11511@thorin> 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> Content-Type: text/plain Date: Tue, 02 Sep 2008 00:46:11 +0200 Message-Id: <1220309171.4115.31.camel@fz.local> Mime-Version: 1.0 X-Mailer: Evolution 2.22.3.1 Content-Transfer-Encoding: 7bit X-Provags-ID: V01U2FsdGVkX1+hQDA5nApNXFVo1ULBIimxruIYyiUuBdm4Wyz WRneaWcSvcgdK/sfr+N4ZHZnK9b3/CGqRGGKEprXOi8Kt4ZlUY jU0JkpDaH/u5FJAgkyvw3f9rmueC0ZY X-detected-kernel: by monty-python.gnu.org: Linux 2.6? (barebone, rare!) 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: Mon, 01 Sep 2008 22:46:16 -0000 Am Dienstag, den 02.09.2008, 00:14 +0200 schrieb Robert Millan: > On Mon, Aug 18, 2008 at 05:05:26PM +0200, Felix Zielcke wrote: > > + unsigned char i, j, k, l; > > I think using unsigned chars to store "integers" is counter-intuitive, and in > some cases possibly dangerous (overflow). I should have probable even used grub_uintN_t types. 16bit should be enough for them I think 255 chars in /dev/mapper/filename could be maybe a problem if people do such weird things :) > > + grub_dev = xmalloc (strlen (os_dev) - strlen ("/dev/mapper/") + 1); > > + > > + j = sizeof ("/dev/mapper/") -1; > > ^ > > Missing space here :-) Yep and I even corrected this already in my last patch http://lists.gnu.org/archive/html/grub-devel/2008-08/msg00523.htmlhttp://lists.gnu.org/archive/html/grub-devel/2008-08/msg00523.html I'm now not that sure if that #define k would be okay, seems like Vesa isn't liking macros. But I could just use const for that too. > > + for (i = 0, k = 0; i < l; i++) > > + { > > + grub_dev[k] = os_dev[j + i]; > > + k++; > > i already counts from 0 and increments by-one. Can it be used instead of k? Oh I just noticed again that in my last patch it's now j i is used as the source count and get's incremented twice for a dash so one dash is skipped. whereas k (new j) is used as the destination count so it's always only incremented by one. That's the whole problem, skip the next dash on a dash but don't skip a single dash. It's always /dev/mapper/vg-lv but if vg and lv part has a single dash then it's for example v--g-l--v for grub this has to be v-g-l-v > The rest of the code I mostly don't understand well. If you feel confident > that it's right, I suggest you check it in unless someone else also wants to > review it. > Honestly I'm happy that the LVM part seems to work and didn't introduce any problem. I'm too lazy now to make a new patch and go sleeping now. But the changes I do make tomorrow now on the last patch above is making k a const instead of #define, i think it just looks better and using grub_uint16_t for all 4 variables. I hope that Marco could have a quick look over it especially the changelog part :) -- Felix Zielcke