* Test command @ 2009-02-13 11:39 phcoder 2009-02-13 11:39 ` [PATCH] " phcoder 0 siblings, 1 reply; 8+ messages in thread From: phcoder @ 2009-02-13 11:39 UTC (permalink / raw) To: The development of GRUB 2 Hello. Here is an implementation of bash-like "test" command. Many file tests are omitted because they are useless in grub (e.g. -c test). I also added 3 extension: lexicographical comparing, prefixed -gt and -lt (it skips common prefix. Useful for comparing versions. e.g. [ vmlinuz-3 -plt vmlinuz-11 ] is true) and biased -nt/-ot which adds s specified amount of seconds to mtime. Regards Vladimir 'phcoder' Serbinenko ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] Test command 2009-02-13 11:39 Test command phcoder @ 2009-02-13 11:39 ` phcoder 2009-04-10 22:18 ` phcoder 0 siblings, 1 reply; 8+ messages in thread From: phcoder @ 2009-02-13 11:39 UTC (permalink / raw) To: The development of GRUB 2 [-- Attachment #1: Type: text/plain, Size: 490 bytes --] Sorry forgot to attach the file phcoder wrote: > Hello. Here is an implementation of bash-like "test" command. Many file > tests are omitted because they are useless in grub (e.g. -c test). I > also added 3 extension: lexicographical comparing, prefixed -gt and -lt > (it skips common prefix. Useful for comparing versions. e.g. [ vmlinuz-3 > -plt vmlinuz-11 ] is true) and biased -nt/-ot which adds s specified > amount of seconds to mtime. > Regards > Vladimir 'phcoder' Serbinenko [-- Attachment #2: test.diff --] [-- Type: text/x-patch, Size: 12884 bytes --] Index: commands/test.c =================================================================== --- commands/test.c (revision 1989) +++ commands/test.c (working copy) @@ -23,33 +23,385 @@ #include <grub/misc.h> #include <grub/mm.h> #include <grub/env.h> +#include <grub/fs.h> +#include <grub/device.h> +#include <grub/file.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)) + { + 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], "=") + || !grub_strcmp (args[*argn + 1], "=="))) + { + update_val (grub_strcmp (args[*argn], args[*argn + 2]) == 0); + (*argn) += 3; + continue; + } + + if (*argn + 2 < argc && !grub_strcmp (args[*argn + 1], "!=")) + { + 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], "<")) + { + update_val (grub_strcmp (args[*argn], args[*argn + 2]) < 0); + (*argn) += 3; + continue; + } + + if (*argn + 2 < argc && !grub_strcmp (args[*argn + 1], "<=")) + { + update_val (grub_strcmp (args[*argn], args[*argn + 2]) <= 0); + (*argn) += 3; + continue; + } + + if (*argn + 2 < argc && !grub_strcmp (args[*argn + 1], ">")) + { + update_val (grub_strcmp (args[*argn], args[*argn + 2]) > 0); + (*argn) += 3; + continue; + } + + if (*argn + 2 < argc && !grub_strcmp (args[*argn + 1], ">=")) + { + 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")) + { + 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")) + { + 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")) + { + 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")) + { + 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")) + { + 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")) + { + 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") + || !grub_strcmp (args[*argn + 1], "-plt"))) + { + 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")) + 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) + || !grub_memcmp (args[*argn + 1], "-ot", 3))) + { + 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)) + 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")) + { + 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")) + { + get_fileinfo (args[*argn + 1]); + update_val (file_exists); + (*argn) += 2; + return ret; + } + + if (*argn + 1 < argc && !grub_strcmp (args[*argn], "-f")) + { + 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")) + { + grub_file_t file; + file = grub_file_open (args[*argn + 1]); + update_val (file && grub_file_size (file)); + if (file) + grub_file_close (file); + grub_errno = GRUB_ERR_NONE; + (*argn) += 2; + return ret; + } + + /* string tests */ + if (!grub_strcmp (args[*argn], "-n") && *argn + 1 < argc) + { + update_val (args[*argn + 1][0]); + + (*argn)+=2; + continue; + } + if (!grub_strcmp (args[*argn], "-z") && *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], ")")) + { + (*argn)++; + return ret; + } + /* Recursively invoke if parenthesis */ + if (!grub_strcmp (args[*argn], "(")) + { + (*argn)++; + update_val (test_parse (args, argn, argc)); + continue; + } + + if (!grub_strcmp (args[*argn], "!")) + { + invert = !invert; + (*argn)++; + continue; + } + if (!grub_strcmp (args[*argn], "-a")) + { + /* if current value is 0 second value is to be discarded */ + discard = !ret; + (*argn)++; + continue; + } + if (!grub_strcmp (args[*argn], "-o")) + { + /* if current value is 1 second value is to be discarded */ + discard = ret; + (*argn)++; + continue; + } + + /* To test found. Interpret if as just a string*/ + update_val (args[*argn][0]); + (*argn)++; + } + return ret; +} + static grub_err_t grub_cmd_test (struct grub_arg_list *state __attribute__ ((unused)), int argc, char **args) { - char *eq; - char *eqis; + int argn = 0; - /* XXX: No fancy expression evaluation yet. */ - - if (argc == 0) - return 0; - - eq = grub_strdup (args[0]); - eqis = grub_strchr (eq, '='); - if (! eqis) - return 0; + if (argc >= 1 && !grub_strcmp (args[argc-1], "]")) + argc--; - *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; + return test_parse (args, &argn, argc) ? GRUB_ERR_NONE + : grub_error (GRUB_ERR_TEST_FAILURE, "false"); } @@ -57,9 +409,11 @@ GRUB_MOD_INIT(test) { (void)mod; /* To stop warning. */ - grub_register_command ("[", grub_cmd_test, GRUB_COMMAND_FLAG_CMDLINE, + grub_register_command ("[", grub_cmd_test, GRUB_COMMAND_FLAG_CMDLINE + | GRUB_COMMAND_FLAG_NO_ARG_PARSE, "[ EXPRESSION ]", "Evaluate an expression", 0); - grub_register_command ("test", grub_cmd_test, GRUB_COMMAND_FLAG_CMDLINE, + grub_register_command ("test", grub_cmd_test, GRUB_COMMAND_FLAG_CMDLINE + | GRUB_COMMAND_FLAG_NO_ARG_PARSE, "test EXPRESSION", "Evaluate an expression", 0); } Index: include/grub/fs.h =================================================================== --- include/grub/fs.h (revision 1989) +++ include/grub/fs.h (working copy) @@ -27,13 +27,14 @@ /* Forward declaration is required, because of mutual reference. */ struct grub_file; struct grub_dirhook_info { int dir:1; int mtimeset:1; + int case_insensitive:1; grub_int32_t mtime; }; /* Filesystem descriptor. */ struct grub_fs { Index: fs/fat.c =================================================================== --- fs/fat.c (revision 1989) +++ fs/fat.c (working copy) @@ -594,9 +557,10 @@ struct grub_dirhook_info info; grub_memset (&info, 0, sizeof (info)); info.dir = (dir->attr & GRUB_FAT_ATTR_DIRECTORY); + info.case_insensitive = 1; if (dir->attr & GRUB_FAT_ATTR_VOLUME_ID) return 0; if (*dirname == '\0' && call_hook) return hook (filename, info); Index: fs/hfsplus.c =================================================================== --- fs/hfsplus.c (revision 1989) +++ fs/hfsplus.c (working copy) @@ -898,11 +899,12 @@ enum grub_fshelp_filetype filetype, grub_fshelp_node_t node) { struct grub_dirhook_info info; grub_memset (&info, 0, sizeof (info)); info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR); + info.case_insensitive = !! (filetype & GRUB_FSHELP_CASE_INSENSITIVE); grub_free (node); return hook (filename, info); } #ifndef GRUB_UTIL Index: ChangeLog =================================================================== --- ChangeLog (revision 1989) +++ ChangeLog (working copy) @@ -1,3 +1,13 @@ +2009-02-13 Vladimir Serbinenko <phcoder@gmail.com> + + Test command + + * commands/test.c: rewritten to use bash-like test + * include/grub/fs.h (struct grub_dirhook_info): new field + case_insensitive + * fs/hfsplus.c: declare FS as case-insensitive if necessary + * fs/fat.c: likewise + 2009-02-11 Robert Millan <rmh@aybabtu.com> * util/grub.d/00_header.in: Update old reference to `font' command. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Test command 2009-02-13 11:39 ` [PATCH] " phcoder @ 2009-04-10 22:18 ` phcoder 2009-04-11 10:07 ` Yoshinori K. Okuji 0 siblings, 1 reply; 8+ messages in thread From: phcoder @ 2009-04-10 22:18 UTC (permalink / raw) To: The development of GRUB 2 [-- Attachment #1: Type: text/plain, Size: 710 bytes --] Rediffed. New changelog 2009-04-11 Vladimir Serbinenko <phcoder@gmail.com> Test command * commands/test.c: rewritten to use bash-like test phcoder wrote: > Sorry forgot to attach the file > phcoder wrote: >> Hello. Here is an implementation of bash-like "test" command. Many >> file tests are omitted because they are useless in grub (e.g. -c >> test). I also added 3 extension: lexicographical comparing, prefixed >> -gt and -lt (it skips common prefix. Useful for comparing versions. >> e.g. [ vmlinuz-3 -plt vmlinuz-11 ] is true) and biased -nt/-ot which >> adds s specified amount of seconds to mtime. >> Regards >> Vladimir 'phcoder' Serbinenko > -- Regards Vladimir 'phcoder' Serbinenko [-- Attachment #2: test.diff --] [-- Type: text/x-diff, Size: 10256 bytes --] diff --git a/commands/test.c b/commands/test.c index a9c8281..2d8dedd 100644 --- a/commands/test.c +++ b/commands/test.c @@ -21,33 +21,385 @@ #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)) + { + 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], "=") + || !grub_strcmp (args[*argn + 1], "=="))) + { + update_val (grub_strcmp (args[*argn], args[*argn + 2]) == 0); + (*argn) += 3; + continue; + } + + if (*argn + 2 < argc && !grub_strcmp (args[*argn + 1], "!=")) + { + 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], "<")) + { + update_val (grub_strcmp (args[*argn], args[*argn + 2]) < 0); + (*argn) += 3; + continue; + } + + if (*argn + 2 < argc && !grub_strcmp (args[*argn + 1], "<=")) + { + update_val (grub_strcmp (args[*argn], args[*argn + 2]) <= 0); + (*argn) += 3; + continue; + } + + if (*argn + 2 < argc && !grub_strcmp (args[*argn + 1], ">")) + { + update_val (grub_strcmp (args[*argn], args[*argn + 2]) > 0); + (*argn) += 3; + continue; + } + + if (*argn + 2 < argc && !grub_strcmp (args[*argn + 1], ">=")) + { + 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")) + { + 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")) + { + 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")) + { + 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")) + { + 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")) + { + 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")) + { + 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") + || !grub_strcmp (args[*argn + 1], "-plt"))) + { + 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")) + 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) + || !grub_memcmp (args[*argn + 1], "-ot", 3))) + { + 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)) + 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")) + { + 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")) + { + get_fileinfo (args[*argn + 1]); + update_val (file_exists); + (*argn) += 2; + return ret; + } + + if (*argn + 1 < argc && !grub_strcmp (args[*argn], "-f")) + { + 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")) + { + grub_file_t file; + file = grub_file_open (args[*argn + 1]); + update_val (file && grub_file_size (file)); + if (file) + grub_file_close (file); + grub_errno = GRUB_ERR_NONE; + (*argn) += 2; + return ret; + } + + /* string tests */ + if (!grub_strcmp (args[*argn], "-n") && *argn + 1 < argc) + { + update_val (args[*argn + 1][0]); + + (*argn)+=2; + continue; + } + if (!grub_strcmp (args[*argn], "-z") && *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], ")")) + { + (*argn)++; + return ret; + } + /* Recursively invoke if parenthesis */ + if (!grub_strcmp (args[*argn], "(")) + { + (*argn)++; + update_val (test_parse (args, argn, argc)); + continue; + } + + if (!grub_strcmp (args[*argn], "!")) + { + invert = !invert; + (*argn)++; + continue; + } + if (!grub_strcmp (args[*argn], "-a")) + { + /* if current value is 0 second value is to be discarded */ + discard = !ret; + (*argn)++; + continue; + } + if (!grub_strcmp (args[*argn], "-o")) + { + /* if current value is 1 second value is to be discarded */ + discard = ret; + (*argn)++; + continue; + } + + /* To 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], "]")) + 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; ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] Test command 2009-04-10 22:18 ` phcoder @ 2009-04-11 10:07 ` Yoshinori K. Okuji 2009-04-11 15:11 ` phcoder 0 siblings, 1 reply; 8+ messages in thread From: Yoshinori K. Okuji @ 2009-04-11 10:07 UTC (permalink / raw) To: The development of GRUB 2 On Saturday 11 April 2009 07:18:59 phcoder wrote: > Rediffed. New changelog This time, I comment on all style problems. > diff --git a/commands/test.c b/commands/test.c > index a9c8281..2d8dedd 100644 > --- a/commands/test.c > +++ b/commands/test.c > @@ -21,33 +21,385 @@ > #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*/ Please make a complete sentence by terminating it with a period. Also, put two space characters before finishing the comment. This is written in the GNU Coding Standards. > +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*/ Same here. > +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*/ Please add a space after the asterisk. > + void update_val (int val) > + { > + if (!discard) Please add a space. > + ret = invert ? !val : val; Same here. > + invert = discard = 0; > + } > + > + /* Check if file exists and fetch its information */ Same here. > + void get_fileinfo (char *pathname) > + { > + char *filename, *path; > + char *device_name; > + grub_fs_t fs; > + grub_device_t dev; > + > + /* A hook for iterating directories */ Same here. > + 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)) I prefer to avoid treating the return value of strcmp/strcasecmp/memcmp as boolean, because even experts often make mistakes. Please use "== 0" instead. > + { > + 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 / */ Same here. > + while (*pathname && pathname[grub_strlen (pathname) - 1] == '/') > + pathname[grub_strlen (pathname) - 1] = 0; > + > + /* Split into path and filename*/ Same here. > + filename = grub_strrchr (pathname, '/'); > + if (!filename) Same here. > + { > + path = grub_strdup ("/"); > + filename = pathname; > + } > + else > + { > + filename++; > + path = grub_strdup (pathname); > + path[filename - pathname] = 0; > + } > + > + /* It's the whole device*/ Same here. > + if (!*pathname) Same here. > + { > + file_exists = 1; > + grub_memset (&file_info, 0, sizeof (file_info)); > + /* Root is always a directory */ Same here. > + file_info.dir = 1; > + > + /* Fetch writing time */ Same here. > + 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 */ Same here. > + while (*argn < argc) > + { > + /* First try 3 argument tests */ Ditto. > + /* String tests */ Ditto. > + if (*argn + 2 < argc && (!grub_strcmp (args[*argn + 1], "=") > + || !grub_strcmp (args[*argn + 1], "=="))) Ditto. > + { > + 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. > + continue; > + } > + > + if (*argn + 2 < argc && !grub_strcmp (args[*argn + 1], "!=")) Ditto. > + { > + update_val (grub_strcmp (args[*argn], args[*argn + 2]) != 0); > + (*argn) += 3; > + continue; > + } > + > + /* GRUB extension: lexicographical sorting */ Ditto. > + if (*argn + 2 < argc && !grub_strcmp (args[*argn + 1], "<")) Ditto. > + { > + update_val (grub_strcmp (args[*argn], args[*argn + 2]) < 0); > + (*argn) += 3; > + continue; > + } > + > + if (*argn + 2 < argc && !grub_strcmp (args[*argn + 1], "<=")) Ditto. > + { > + update_val (grub_strcmp (args[*argn], args[*argn + 2]) <= 0); > + (*argn) += 3; > + continue; > + } > + > + if (*argn + 2 < argc && !grub_strcmp (args[*argn + 1], ">")) Ditto. > + { > + update_val (grub_strcmp (args[*argn], args[*argn + 2]) > 0); > + (*argn) += 3; > + continue; > + } > + > + if (*argn + 2 < argc && !grub_strcmp (args[*argn + 1], ">=")) Ditto. > + { > + update_val (grub_strcmp (args[*argn], args[*argn + 2]) >= 0); > + (*argn) += 3; > + continue; > + } > + > + /* Number tests */ Ditto. > + if (*argn + 2 < argc && !grub_strcmp (args[*argn + 1], "-eq")) Ditto. > + { > + 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")) Ditto. > + { > + 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")) Ditto. > + { > + 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")) Ditto. > + { > + 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")) Ditto. > + { > + 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")) Ditto. > + { > + 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*/ Ditto. > + if (*argn + 2 < argc && (!grub_strcmp (args[*argn + 1], "-pgt") > + || !grub_strcmp (args[*argn + 1], "-plt"))) Ditto. > + { > + int i; > + /* Skip common prefix */ Ditto. > + for (i = 0; args[*argn][i] == args[*argn + 2][i] && > args[*argn][i]; + i++); > + > + /*Go the digits back*/ Ditto. > + i--; > + while (grub_isdigit (args[*argn][i]) && i > 0) > + i--; > + i++; > + > + if (!grub_strcmp (args[*argn + 1], "-pgt")) Ditto. > + 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 */ Ditto. > + if (*argn + 2 < argc && (!grub_memcmp (args[*argn + 1], "-nt", 3) > + || !grub_memcmp (args[*argn + 1], "-ot", > 3))) + { Ditto. Getting tired, so I skip the same criticism from here. > + 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)) > + 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")) > + { > + 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")) > + { > + get_fileinfo (args[*argn + 1]); > + update_val (file_exists); > + (*argn) += 2; > + return ret; > + } > + > + if (*argn + 1 < argc && !grub_strcmp (args[*argn], "-f")) > + { > + 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")) > + { > + 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. > + if (file) > + grub_file_close (file); > + grub_errno = GRUB_ERR_NONE; > + (*argn) += 2; > + return ret; > + } > + > + /* string tests */ > + if (!grub_strcmp (args[*argn], "-n") && *argn + 1 < argc) > + { > + update_val (args[*argn + 1][0]); > + > + (*argn)+=2; > + continue; > + } > + if (!grub_strcmp (args[*argn], "-z") && *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], ")")) > + { > + (*argn)++; > + return ret; > + } > + /* Recursively invoke if parenthesis */ > + if (!grub_strcmp (args[*argn], "(")) > + { > + (*argn)++; > + update_val (test_parse (args, argn, argc)); > + continue; > + } > + > + if (!grub_strcmp (args[*argn], "!")) > + { > + invert = !invert; > + (*argn)++; > + continue; > + } > + if (!grub_strcmp (args[*argn], "-a")) > + { > + /* if current value is 0 second value is to be discarded */ > + discard = !ret; > + (*argn)++; > + continue; > + } > + if (!grub_strcmp (args[*argn], "-o")) > + { > + /* if current value is 1 second value is to be discarded */ > + discard = ret; > + (*argn)++; > + continue; > + } > + > + /* To 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], "]")) > + 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; Regards, Okuji ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Test command 2009-04-11 10:07 ` Yoshinori K. Okuji @ 2009-04-11 15:11 ` phcoder 2009-04-14 15:55 ` Yoshinori K. Okuji 0 siblings, 1 reply; 8+ messages in thread From: phcoder @ 2009-04-11 15:11 UTC (permalink / raw) To: The development of GRUB 2 [-- Attachment #1: Type: text/plain, Size: 1114 bytes --] 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 [-- Attachment #2: test.diff --] [-- Type: text/x-diff, Size: 10450 bytes --] 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; ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] Test command 2009-04-11 15:11 ` phcoder @ 2009-04-14 15:55 ` Yoshinori K. Okuji 2009-04-16 13:15 ` phcoder 0 siblings, 1 reply; 8+ messages in thread From: Yoshinori K. Okuji @ 2009-04-14 15:55 UTC (permalink / raw) To: The development of GRUB 2 On Sunday 12 April 2009 00:11:45 phcoder wrote: > 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 OK. > > > 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. BTW, I think you can simplify test_parse. For example, you write "if (*argn + 2 < argc ...)" many times, but it should be possible to test this condition only once per loop. Regards, Okuji ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Test command 2009-04-14 15:55 ` Yoshinori K. Okuji @ 2009-04-16 13:15 ` phcoder 2009-04-25 12:29 ` Vladimir Serbinenko 0 siblings, 1 reply; 8+ messages in thread From: phcoder @ 2009-04-16 13:15 UTC (permalink / raw) To: The development of GRUB 2 [-- Attachment #1: Type: text/plain, Size: 485 bytes --] > BTW, I think you can simplify test_parse. For example, you write "if (*argn + > 2 < argc ...)" many times, but it should be possible to test this condition > only once per loop. Optimised. Perhaps compiler optimised this anyway but it made code more readable > > Regards, > Okuji > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel -- Regards Vladimir 'phcoder' Serbinenko [-- Attachment #2: test.diff --] [-- Type: text/x-diff, Size: 10585 bytes --] diff --git a/commands/test.c b/commands/test.c index a9c8281..8a15d39 100644 --- a/commands/test.c +++ b/commands/test.c @@ -21,33 +21,390 @@ #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. */ + if (*argn + 2 < argc) + { + /* String tests. */ + if (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 (grub_strcmp (args[*argn + 1], "!=") == 0) + { + update_val (grub_strcmp (args[*argn], args[*argn + 2]) != 0); + (*argn) += 3; + continue; + } + + /* GRUB extension: lexicographical sorting. */ + if (grub_strcmp (args[*argn + 1], "<") == 0) + { + update_val (grub_strcmp (args[*argn], args[*argn + 2]) < 0); + (*argn) += 3; + continue; + } + + if (grub_strcmp (args[*argn + 1], "<=") == 0) + { + update_val (grub_strcmp (args[*argn], args[*argn + 2]) <= 0); + (*argn) += 3; + continue; + } + + if (grub_strcmp (args[*argn + 1], ">") == 0) + { + update_val (grub_strcmp (args[*argn], args[*argn + 2]) > 0); + (*argn) += 3; + continue; + } + + if (grub_strcmp (args[*argn + 1], ">=") == 0) + { + update_val (grub_strcmp (args[*argn], args[*argn + 2]) >= 0); + (*argn) += 3; + continue; + } + + /* Number tests. */ + if (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 (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 (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 (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 (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 (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 (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 (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. */ + if (*argn + 1 < argc) + { + /* File tests. */ + if (grub_strcmp (args[*argn], "-d") == 0) + { + get_fileinfo (args[*argn + 1]); + update_val (file_exists && file_info.dir); + (*argn) += 2; + return ret; + } + + if (grub_strcmp (args[*argn], "-e") == 0) + { + get_fileinfo (args[*argn + 1]); + update_val (file_exists); + (*argn) += 2; + return ret; + } + + if (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 (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) + { + update_val (args[*argn + 1][0]); + + (*argn) += 2; + continue; + } + if (grub_strcmp (args[*argn], "-z") == 0) + { + 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; ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] Test command 2009-04-16 13:15 ` phcoder @ 2009-04-25 12:29 ` Vladimir Serbinenko 0 siblings, 0 replies; 8+ messages in thread From: Vladimir Serbinenko @ 2009-04-25 12:29 UTC (permalink / raw) To: The development of GRUB 2 [-- Attachment #1: Type: text/plain, Size: 590 bytes --] commited On Thu, Apr 16, 2009 at 3:15 PM, phcoder <phcoder@gmail.com> wrote: > BTW, I think you can simplify test_parse. For example, you write "if (*argn >> + 2 < argc ...)" many times, but it should be possible to test this >> condition only once per loop. >> > Optimised. Perhaps compiler optimised this anyway but it made code more > readable > > >> Regards, >> Okuji >> >> >> _______________________________________________ >> Grub-devel mailing list >> Grub-devel@gnu.org >> http://lists.gnu.org/mailman/listinfo/grub-devel >> > > > -- > > Regards > Vladimir 'phcoder' Serbinenko > [-- Attachment #2: Type: text/html, Size: 1422 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-04-25 12:29 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-02-13 11:39 Test command phcoder 2009-02-13 11:39 ` [PATCH] " phcoder 2009-04-10 22:18 ` phcoder 2009-04-11 10:07 ` Yoshinori K. Okuji 2009-04-11 15:11 ` phcoder 2009-04-14 15:55 ` Yoshinori K. Okuji 2009-04-16 13:15 ` phcoder 2009-04-25 12:29 ` Vladimir Serbinenko
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.