test -e patch

[adrian15 <adrian15@raulete.net>]

[-- Attachment #1: Type: text/plain, Size: 3441 bytes --]

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;
> 
> 
> }
> 
> 
> \f
> 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


[-- Attachment #2: test_e.patch --]
[-- Type: text/x-patch, Size: 1839 bytes --]

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 <grub/misc.h>
 #include <grub/mm.h>
 #include <grub/env.h>
+#include <grub/file.h>
+
+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;
 }