Re: [PATCH] Test command

[phcoder]

Updated. Same changelog
> > +       {
> > +         update_val (grub_strcmp (args[*argn], args[*argn + 2]) == 0);
> > +         (*argn) += 3;
>
> I myself feel that these parentheses are redundant, but I don't know how 
> others think. For C programmers, it is well known that * has a very high 
> priority.
These parenthesis are necessary if doing sth like
(*argn)++
since ++ and += are logically and visually similar I prefer to put the 
parenthesis
> Getting tired, so I skip the same criticism from here.
Actually it would have been enough to say "same applies further on in patch"
> > +      if (*argn + 1 < argc && !grub_strcmp (args[*argn], "-s"))
> > +       {
> > +         grub_file_t file;
> > +         file = grub_file_open (args[*argn + 1]);
> > +         update_val (file && grub_file_size (file));
>
> This is not very safe, because grub_file_size returns grub_off_t which is a 
> 64-bit unsigned int. By converting it into 32-bit signed int implicitly, the 
> result can be zero, even when the size is not zero. So it is better to say 
> explicitly, != 0.

--

Regards
Vladimir 'phcoder' Serbinenko

diff --git a/commands/test.c b/commands/test.c
index a9c8281..1ccfe48 100644
--- a/commands/test.c
+++ b/commands/test.c
@@ -21,33 +21,384 @@
 #include <grub/misc.h>
 #include <grub/mm.h>
 #include <grub/env.h>
+#include <grub/fs.h>
+#include <grub/device.h>
+#include <grub/file.h>
 #include <grub/command.h>
 
+/* A simple implementation for signed numbers. */
+static int
+grub_strtosl (char *arg, char **end, int base)
+{
+  if (arg[0] == '-')
+    return -grub_strtoul (arg + 1, end, base);
+  return grub_strtoul (arg, end, base);
+}
+
+/* Parse a test expression startion from *argn. */
+static int
+test_parse (char **args, int *argn, int argc)
+{
+  int ret = 0, discard = 0, invert = 0;
+  int file_exists;
+  struct grub_dirhook_info file_info;
+
+  auto void update_val (int val);
+  auto void get_fileinfo (char *pathname);
+
+  /* Take care of discarding and inverting. */
+  void update_val (int val)
+  {
+    if (! discard)
+      ret = invert ? ! val : val;
+    invert = discard = 0;
+  }
+
+  /* Check if file exists and fetch its information. */
+  void get_fileinfo (char *pathname)
+  {
+    char *filename, *path;
+    char *device_name;
+    grub_fs_t fs;
+    grub_device_t dev;
+
+    /* A hook for iterating directories. */
+    auto int find_file (const char *cur_filename, 
+                       struct grub_dirhook_info info);
+    int find_file (const char *cur_filename, struct grub_dirhook_info info)
+    {
+      if ((info.case_insensitive ? grub_strcasecmp (cur_filename, filename)
+          : grub_strcmp (cur_filename, filename)) == 0)
+       {
+         file_info = info;
+         file_exists = 1;
+         return 1;
+       }
+      return 0;
+    }
+    
+    file_exists = 0;
+    device_name = grub_file_get_device_name (pathname);
+    dev = grub_device_open (device_name);
+    if (! dev)
+      {
+       grub_free (device_name);
+       return;
+      }
+
+    fs = grub_fs_probe (dev);
+    path = grub_strchr (pathname, ')');
+    if (! path)
+      path = pathname;
+    else
+      path++;
+    
+    /* Remove trailing '/'. */
+    while (*pathname && pathname[grub_strlen (pathname) - 1] == '/')
+      pathname[grub_strlen (pathname) - 1] = 0;
+
+    /* Split into path and filename. */
+    filename = grub_strrchr (pathname, '/');
+    if (! filename)
+      {
+       path = grub_strdup ("/");
+       filename = pathname;
+      }
+    else
+      {
+       filename++;
+       path = grub_strdup (pathname);
+       path[filename - pathname] = 0;
+      }
+
+    /* It's the whole device. */
+    if (! *pathname)
+      {
+       file_exists = 1;
+       grub_memset (&file_info, 0, sizeof (file_info));
+       /* Root is always a directory. */
+       file_info.dir = 1;
+
+       /* Fetch writing time. */
+       file_info.mtimeset = 0;
+       if (fs->mtime)
+         {
+           if (! fs->mtime (dev, &file_info.mtime))
+             file_info.mtimeset = 1;
+           grub_errno = GRUB_ERR_NONE;
+         }
+      }
+    else
+      (fs->dir) (dev, path, find_file);
+
+    grub_device_close (dev); 
+    grub_free (path);
+    grub_free (device_name);
+  }
+
+  /* Here we have the real parsing. */
+  while (*argn < argc)
+    {
+      /* First try 3 argument tests. */
+      /* String tests. */
+      if (*argn + 2 < argc && (grub_strcmp (args[*argn + 1], "=") == 0
+                              || grub_strcmp (args[*argn + 1], "==") == 0))
+       {
+         update_val (grub_strcmp (args[*argn], args[*argn + 2]) == 0);
+         (*argn) += 3;
+         continue;
+       }
+
+      if (*argn + 2 < argc && grub_strcmp (args[*argn + 1], "!=") == 0)
+       {
+         update_val (grub_strcmp (args[*argn], args[*argn + 2]) != 0);
+         (*argn) += 3;
+         continue;
+       }
+
+      /* GRUB extension: lexicographical sorting. */
+      if (*argn + 2 < argc && grub_strcmp (args[*argn + 1], "<") == 0)
+       {
+         update_val (grub_strcmp (args[*argn], args[*argn + 2]) < 0);
+         (*argn) += 3;
+         continue;
+       }
+
+      if (*argn + 2 < argc && grub_strcmp (args[*argn + 1], "<=") == 0)
+       {
+         update_val (grub_strcmp (args[*argn], args[*argn + 2]) <= 0);
+         (*argn) += 3;
+         continue;
+       }
+
+      if (*argn + 2 < argc && grub_strcmp (args[*argn + 1], ">") == 0)
+       {
+         update_val (grub_strcmp (args[*argn], args[*argn + 2]) > 0);
+         (*argn) += 3;
+         continue;
+       }
+
+      if (*argn + 2 < argc && grub_strcmp (args[*argn + 1], ">=") == 0)
+       {
+         update_val (grub_strcmp (args[*argn], args[*argn + 2]) >= 0);
+         (*argn) += 3;
+         continue;
+       }
+
+      /* Number tests. */
+      if (*argn + 2 < argc && grub_strcmp (args[*argn + 1], "-eq") == 0)
+       {
+         update_val (grub_strtosl (args[*argn], 0, 0) 
+                     == grub_strtosl (args[*argn + 2], 0, 0));
+         (*argn) += 3;
+         continue;
+       }
+
+      if (*argn + 2 < argc && grub_strcmp (args[*argn + 1], "-ge") == 0)
+       {
+         update_val (grub_strtosl (args[*argn], 0, 0) 
+                     >= grub_strtosl (args[*argn + 2], 0, 0));
+         (*argn) += 3;
+         continue;
+       }
+
+      if (*argn + 2 < argc && grub_strcmp (args[*argn + 1], "-gt") == 0)
+       {
+         update_val (grub_strtosl (args[*argn], 0, 0) 
+                     > grub_strtosl (args[*argn + 2], 0, 0));
+         (*argn) += 3;
+         continue;
+       }
+
+      if (*argn + 2 < argc && grub_strcmp (args[*argn + 1], "-le") == 0)
+       {
+         update_val (grub_strtosl (args[*argn], 0, 0) 
+                     <= grub_strtosl (args[*argn + 2], 0, 0));
+         (*argn) += 3;
+         continue;
+       }
+
+      if (*argn + 2 < argc && grub_strcmp (args[*argn + 1], "-lt") == 0)
+       {
+         update_val (grub_strtosl (args[*argn], 0, 0) 
+                     < grub_strtosl (args[*argn + 2], 0, 0));
+         (*argn) += 3;
+         continue;
+       }
+
+      if (*argn + 2 < argc && grub_strcmp (args[*argn + 1], "-ne") == 0)
+       {
+         update_val (grub_strtosl (args[*argn], 0, 0) 
+                     != grub_strtosl (args[*argn + 2], 0, 0));
+         (*argn) += 3;
+         continue;
+       }
+
+      /* GRUB extension: compare numbers skipping prefixes. 
+        Useful for comparing versions. E.g. vmlinuz-2 -plt vmlinuz-11. */
+      if (*argn + 2 < argc && (grub_strcmp (args[*argn + 1], "-pgt") == 0
+                              || grub_strcmp (args[*argn + 1], "-plt") == 0))
+       {
+         int i;
+         /* Skip common prefix. */
+         for (i = 0; args[*argn][i] == args[*argn + 2][i] && args[*argn][i];
+              i++);
+
+         /* Go the digits back. */
+         i--;
+         while (grub_isdigit (args[*argn][i]) && i > 0)
+           i--;
+         i++;
+
+         if (grub_strcmp (args[*argn + 1], "-pgt") == 0)
+           update_val (grub_strtoul (args[*argn] + i, 0, 0) 
+                       > grub_strtoul (args[*argn + 2] + i, 0, 0));
+         else
+           update_val (grub_strtoul (args[*argn] + i, 0, 0) 
+                       < grub_strtoul (args[*argn + 2] + i, 0, 0));
+         (*argn) += 3;
+         continue;
+       }
+
+      /* -nt and -ot tests. GRUB extension: when doing -?t<bias> bias 
+        will be added to the first mtime. */
+      if (*argn + 2 < argc && (grub_memcmp (args[*argn + 1], "-nt", 3) == 0
+                              || grub_memcmp (args[*argn + 1], "-ot", 3) == 0))
+       {
+         struct grub_dirhook_info file1;
+         int file1exists;
+         int bias = 0;
+
+         /* Fetch fileinfo */
+         get_fileinfo (args[*argn]);
+         file1 = file_info;
+         file1exists = file_exists;
+         get_fileinfo (args[*argn + 2]);
+
+         if (args[*argn + 1][3])
+           bias = grub_strtosl (args[*argn + 1] + 3, 0, 0);
+
+         if (grub_memcmp (args[*argn + 1], "-nt", 3) == 0)
+           update_val ((file1exists && ! file_exists)
+                       || (file1.mtimeset && file_info.mtimeset
+                           && file1.mtime + bias > file_info.mtime));
+         else
+           update_val ((! file1exists && file_exists)
+                       || (file1.mtimeset && file_info.mtimeset
+                           && file1.mtime + bias < file_info.mtime));
+         (*argn) += 3;
+         continue;
+       }
+
+      /* Two-argument tests. */
+      /* file tests. */
+      if (*argn + 1 < argc && grub_strcmp (args[*argn], "-d") == 0)
+       {
+         get_fileinfo (args[*argn + 1]);
+         update_val (file_exists && file_info.dir);
+         (*argn) += 2;
+         return ret;
+       }
+
+      if (*argn + 1 < argc && grub_strcmp (args[*argn], "-e") == 0)
+       {
+         get_fileinfo (args[*argn + 1]);
+         update_val (file_exists);
+         (*argn) += 2;
+         return ret;
+       }
+
+      if (*argn + 1 < argc && grub_strcmp (args[*argn], "-f") == 0)
+       {
+         get_fileinfo (args[*argn + 1]);
+         /* FIXME: check for other types. */
+         update_val (file_exists && ! file_info.dir);
+         (*argn) += 2;
+         return ret;
+       }
+
+      if (*argn + 1 < argc && grub_strcmp (args[*argn], "-s") == 0)
+       {
+         grub_file_t file;
+         file = grub_file_open (args[*argn + 1]);
+         update_val (file && (grub_file_size (file) != 0));
+         if (file)
+           grub_file_close (file);
+         grub_errno = GRUB_ERR_NONE;
+         (*argn) += 2;
+         return ret;
+       }
+
+      /* string tests. */
+      if (grub_strcmp (args[*argn], "-n") == 0 && *argn + 1 < argc)
+       {
+         update_val (args[*argn + 1][0]);
+         
+         (*argn) += 2;
+         continue;
+       }
+      if (grub_strcmp (args[*argn], "-z") == 0 && *argn + 1 < argc)
+       {
+         update_val (! args[*argn + 1][0]);
+         (*argn) += 2;
+         continue;
+       }
+
+      /* Special modifiers. */
+      
+      /* End of expression. return to parent. */
+      if (grub_strcmp (args[*argn], ")") == 0)
+       {
+         (*argn)++;
+         return ret;
+       }
+      /* Recursively invoke if parenthesis. */
+      if (grub_strcmp (args[*argn], "(") == 0)
+       {
+         (*argn)++;
+         update_val (test_parse (args, argn, argc));
+         continue;
+       }
+      
+      if (grub_strcmp (args[*argn], "!") == 0)
+       {
+         invert = ! invert;
+         (*argn)++;
+         continue;
+       }
+      if (grub_strcmp (args[*argn], "-a") == 0)
+       {
+         /* if current value is 0 second value is to be discarded */
+         discard = ! ret;
+         (*argn)++;
+         continue;
+       }
+      if (grub_strcmp (args[*argn], "-o") == 0)
+       {
+         /* If current value is 1 second value is to be discarded. */
+         discard = ret;
+         (*argn)++;
+         continue;
+       }
+
+      /* No test found. Interpret if as just a string. */
+      update_val (args[*argn][0]);
+      (*argn)++;
+    }
+  return ret;
+}
+
 static grub_err_t
 grub_cmd_test (grub_command_t cmd __attribute__ ((unused)),
               int argc, char **args)
 {
-  char *eq;
-  char *eqis;
-
-  /* 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;
+  int argn = 0;
+
+  if (argc >= 1 && grub_strcmp (args[argc - 1], "]") == 0)
+    argc--;
+
+  return test_parse (args, &argn, argc) ? GRUB_ERR_NONE 
+    : grub_error (GRUB_ERR_TEST_FAILURE, "false");
 }
 
 static grub_command_t cmd_1, cmd_2;