From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1JOZq9-0001R1-On for mharc-grub-devel@gnu.org; Mon, 11 Feb 2008 09:41:01 -0500 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1JOZq8-0001Qj-O1 for grub-devel@gnu.org; Mon, 11 Feb 2008 09:41:00 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1JOZq3-0001Pc-49 for grub-devel@gnu.org; Mon, 11 Feb 2008 09:40:59 -0500 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JOZq2-0001PK-Ua for grub-devel@gnu.org; Mon, 11 Feb 2008 09:40:54 -0500 Received: from aybabtu.com ([69.60.117.155]) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1JOZq2-00020l-Ky for grub-devel@gnu.org; Mon, 11 Feb 2008 09:40:54 -0500 Received: from [192.168.10.6] (helo=thorin) by aybabtu.com with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.63) (envelope-from ) id 1JOZpy-0006LH-GM for grub-devel@gnu.org; Mon, 11 Feb 2008 15:40:53 +0100 Received: from rmh by thorin with local (Exim 4.63) (envelope-from ) id 1JOZoF-0005hG-JR for grub-devel@gnu.org; Mon, 11 Feb 2008 15:39:03 +0100 Date: Mon, 11 Feb 2008 15:39:03 +0100 From: Robert Millan To: The development of GRUB 2 Message-ID: <20080211143903.GA21126@thorin> References: <47B00CFB.4070304@leat.rub.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <47B00CFB.4070304@leat.rub.de> Organization: free as in freedom X-Message-Flag: Worried about Outlook viruses? Switch to Thunderbird! www.mozilla.com/thunderbird X-Debbugs-No-Ack: true User-Agent: Mutt/1.5.13 (2006-08-11) X-detected-kernel: by monty-python.gnu.org: Genre and OS details not recognized. Subject: Re: [PATCH] Add option to grub-probe to accept system devices as arguments 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, 11 Feb 2008 14:41:00 -0000 On Mon, Feb 11, 2008 at 09:53:15AM +0100, Fabian Greffrath wrote: > The following patch adds to grub-probe the ability to accept system devices > as > arguments and e.g. convert between system devices and GRUB drives. > > This patch is improved over the one from my previous posting and has some > minor > issues fixed. Please use this version instead. It is all my work. Hi Fabian, Some comments on this one. > ???2008-02-11 Fabian Greffrath > > * util/grub-probe.c: Add new parameter '-d, --device'. If this is set, > grub-probe expects the given argument to be a block device. All of the > '--target' parameters work with this option as well. If the '--device' > parameter is not set, grub-probe will work as before. You need to list every function or variable separately. E.g: * util/grub-probe.c (argument_is_device): New variable. (probe): Blah blah. (main): Etc. If you use the -p option to diff it'll be easier to check it just by reading the patch. > +char * > +grub_util_check_block_device (const char *blk_dev) > +{ > + struct stat st; > + > + if (stat (blk_dev, &st) < 0) > + grub_util_error ("Cannot stat `%s'", blk_dev); > + > + if (S_ISBLK (st.st_mode)) > + return strdup (blk_dev); > + else > + return 0; > +} Is there really a need to strdup() it? > diff -Naru grub2-1.96+20080203~/util/grub-probe.c grub2-1.96+20080203/util/grub-probe.c > --- grub2-1.96+20080203~/util/grub-probe.c 2008-01-25 23:33:57.000000000 +0100 > +++ grub2-1.96+20080203/util/grub-probe.c 2008-02-08 12:50:13.000000000 +0100 > @@ -50,6 +50,7 @@ > }; > > int print = PRINT_FS; > +static unsigned int argument_is_device = 0; > > void > grub_putchar (int c) > @@ -84,9 +85,18 @@ > int abstraction_type; > grub_device_t dev = NULL; > > - device_name = grub_guess_root_device (path); > + if (argument_is_device) > + device_name = grub_util_check_block_device (path); > + else > + device_name = grub_guess_root_device (path); I find it confusing that you change the meaning of the `path' variable without renaming it. Also, the variable that describes what `path' is (argument_is_device) is passed separately. What would you think of a scheme where both are passed as strings and either can be NULL? E.g. probe (char *path, char *device_name) { if (path == NULL) { if (grub_util_check_block_device (device_name) == -1) return -1; } else device_name = grub_guess_root_device (path); } > -Usage: grub-probe [OPTION]... PATH\n\ > +Usage: grub-probe [OPTION]... [PATH|DEVICE]\n\ > \n\ > -Probe device information for a given path.\n\ > +Probe device information for a given path or device.\n\ I suspect advertising it here might lead users to think they can pass a device to it, without caring about ... > + -d, --device given argument is a system device, not a path\n\ ... this option. -- Robert Millan I know my rights; I want my phone call! What use is a phone call… if you are unable to speak? (as seen on /.)