From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1HuB9T-0005zO-AF for mharc-grub-devel@gnu.org; Fri, 01 Jun 2007 13:43:03 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1HuB9R-0005zJ-Te for grub-devel@gnu.org; Fri, 01 Jun 2007 13:43:01 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1HuB9Q-0005z7-4V for grub-devel@gnu.org; Fri, 01 Jun 2007 13:43:01 -0400 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1HuB9Q-0005z4-0c for grub-devel@gnu.org; Fri, 01 Jun 2007 13:43:00 -0400 Received: from 245.red-80-24-127.staticip.rima-tde.net ([80.24.127.245] helo=manazas.ensanjose.net) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1HuB9J-0005QK-3W for grub-devel@gnu.org; Fri, 01 Jun 2007 13:42:59 -0400 Received: from localhost (manazas.ensanjose.net [127.0.0.1]) by manazas.ensanjose.net (Postfix) with ESMTP id 27E22D444D for ; Fri, 1 Jun 2007 19:42:48 +0200 (CEST) Received: from manazas.ensanjose.net ([127.0.0.1]) by localhost (manazas.ensanjose.net [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 02676-04 for ; Fri, 1 Jun 2007 19:42:48 +0200 (CEST) Received: from [192.168.1.5] (67.Red-83-34-180.dynamicIP.rima-tde.net [83.34.180.67]) by manazas.ensanjose.net (Postfix) with ESMTP id E1581D32AE for ; Fri, 1 Jun 2007 19:42:46 +0200 (CEST) Message-ID: <4665F4F8.2040305@raulete.net> Date: Wed, 06 Jun 2007 01:42:48 +0200 From: adrian15 User-Agent: Mozilla Thunderbird 1.0.7 (Windows/20050923) X-Accept-Language: en-us, en MIME-Version: 1.0 To: grub-devel@gnu.org Content-Type: multipart/mixed; boundary="------------070006000405070009050301" X-detected-kernel: Linux 2.4-2.6 Subject: test -e patch 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, 01 Jun 2007 17:43:02 -0000 This is a multi-part message in MIME format. --------------070006000405070009050301 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Attached you will find the patch adding test -e support for grub2. This is my first patch. I have compiled it without no errors. However as long as the grub2.tar.gz that Marco gave me did not have any documentation about how to create a floppy (or at least I did not manage to find it) I have not tested it. Could you please test it? I have applied the GNU Coding Standards Style on the patch although the copy-pasted code that you will find in the mail body has not the GCS applied. Here there are some doubts: > static const struct grub_arg_option options[] = > { > {"file", 'e', 0, "test if a file exists", 0, 0}, > {0, 0, 0, 0, 0, 0} > }; Is this correct? What's that last line for? Is it compulsory ? Should I write "Test if a file exists" instead of "test if a file exists" or "FILE exists"? > > static void > test_file_exists (const char *key) Do you prefer another name like: file_exists_test ? > { > grub_file_t file; > > file = grub_file_open (key); > if (file) > { > grub_printf (" %s", key); Should I prompt the existent file or not? Should I prompt %s exists maybe ? Should I prompt an \n also? > grub_file_close (file); > grub_errno = GRUB_ERR_NONE; > } > else > { > grub_error (GRUB_ERR_FILE_NOT_FOUND, "File does not exists."); How the hell should I treat grub errors. Maybe the test_file_exists has to return a static grub_err_t and then from grub_cmd_test I should call it like this: return (test_file_exists (args[0])) so that the error propagates ? What are the error polices ? > } > } > > > static grub_err_t > grub_cmd_test (struct grub_arg_list *state __attribute__ ((unused)), int argc, > char **args) > > { > > char *eq; > char *eqis; > > if (state[0].set) I suppose this refers to the first line of static const struct grub_arg_option options and thus as -e is the first option and if its set we should run the correspondent function. One question. Should we put the test-e function into a separate file or not ? > test_file_exists (args[0]); > else > { > /* XXX: No fancy expression evaluation yet. */ > > if (argc == 0) > return 0; > > eq = grub_strdup (args[0]); > eqis = grub_strchr (eq, '='); > if (! eqis) > return 0; > > *eqis = '\0'; > eqis++; > /* Check an expression in the form `A=B'. */ > if (grub_strcmp (eq, eqis)) > grub_error (GRUB_ERR_TEST_FAILURE, "false"); > grub_free (eq); > } > > return grub_errno; > > > } > > > > GRUB_MOD_INIT(test) > { > (void)mod; /* To stop warning. */ > grub_register_command ("[", grub_cmd_test, GRUB_COMMAND_FLAG_CMDLINE, > "[ EXPRESSION ]", "Evaluate an expression", 0); > grub_register_command ("test", grub_cmd_test, GRUB_COMMAND_FLAG_CMDLINE, > "test EXPRESSION", "Evaluate an expression", 0); > } I understand this register commands. I suppose this information is read from the help command or it isn't ? Or maybe it also reads from: static const struct grub_arg_option options ? The question is if the user will see the -e, -f or other options when querying the test command help or not ? > > GRUB_MOD_FINI(test) > { > grub_unregister_command ("["); > grub_unregister_command ("test"); > } That's all for my first compiled patch. I hope that Marco_g is happy :). But you know Marco_g I will continue my non-sense grub legacy development too. ;) adrian15 --------------070006000405070009050301 Content-Type: text/x-patch; name="test_e.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="test_e.patch" diff -urN grub2_2007_05_31_original/commands/test.c grub2_2007_05_31/commands/test.c --- grub2_2007_05_31_original/commands/test.c 2005-11-13 16:47:08.000000000 +0100 +++ grub2_2007_05_31/commands/test.c 2007-06-01 12:13:11.000000000 +0200 @@ -24,32 +24,64 @@ #include #include #include +#include + +static const struct grub_arg_option options[] = + { + {"file", 'e', 0, "Test if a file exists", 0, 0}, + {0, 0, 0, 0, 0, 0} + }; + +static void +test_file_exists (const char *key) +{ + grub_file_t file; + + file = grub_file_open (key); + if (file) + { + grub_printf (" %s", key); + grub_file_close (file); + grub_errno = GRUB_ERR_NONE; + } + else + { + grub_error (GRUB_ERR_FILE_NOT_FOUND, "File does not exists."); + } +} + static grub_err_t grub_cmd_test (struct grub_arg_list *state __attribute__ ((unused)), int argc, char **args) { + char *eq; char *eqis; - /* XXX: No fancy expression evaluation yet. */ + if (state[0].set) + test_file_exists (args[0]); + else + { + /* XXX: No fancy expression evaluation yet. */ - if (argc == 0) - return 0; + if (argc == 0) + return 0; + + eq = grub_strdup (args[0]); + eqis = grub_strchr (eq, '='); + if (! eqis) + return 0; + + *eqis = '\0'; + eqis++; + /* Check an expression in the form `A=B'. */ + if (grub_strcmp (eq, eqis)) + grub_error (GRUB_ERR_TEST_FAILURE, "false"); + grub_free (eq); + } - eq = grub_strdup (args[0]); - eqis = grub_strchr (eq, '='); - if (! eqis) - return 0; - - *eqis = '\0'; - eqis++; - /* Check an expression in the form `A=B'. */ - if (grub_strcmp (eq, eqis)) - grub_error (GRUB_ERR_TEST_FAILURE, "false"); - grub_free (eq); - return grub_errno; } --------------070006000405070009050301--