diff options
-rw-r--r-- | discover/grub2/builtins.c | 33 | ||||
-rw-r--r-- | discover/parser.c | 30 | ||||
-rw-r--r-- | discover/parser.h | 20 | ||||
-rw-r--r-- | test/parser/Makefile.am | 1 | ||||
-rw-r--r-- | test/parser/parser-test.h | 5 | ||||
-rw-r--r-- | test/parser/test-grub2-test-file-ops.c | 63 | ||||
-rw-r--r-- | test/parser/utils.c | 29 |
7 files changed, 149 insertions, 32 deletions
diff --git a/discover/grub2/builtins.c b/discover/grub2/builtins.c index 6ada2a6..8bff732 100644 --- a/discover/grub2/builtins.c +++ b/discover/grub2/builtins.c @@ -124,43 +124,58 @@ static int builtin_search(struct grub2_script *script, return 0; } +/* Note that GRUB does not follow symlinks in evaluating all file + * tests but -s, unlike below. However, it seems like a bad idea to + * emulate GRUB's behavior (e.g., it would take extra work), so we + * implement the behavior that coreutils' test binary has. */ static bool builtin_test_op_file(struct grub2_script *script, char op, const char *file) { bool result; - int len, rc; - char *buf; + int rc; + struct stat statbuf; - rc = parser_request_file(script->ctx, script->ctx->device, - file, &buf, &len); + rc = parser_stat_path(script->ctx, script->ctx->device, + file, &statbuf); if (rc) return false; switch (op) { case 's': /* -s: return true if file exists and has non-zero size */ - result = len > 0; + result = statbuf.st_size > 0; break; case 'f': - /* -f: return true if file exists */ - result = true; + /* -f: return true if file exists and is not a directory. This is + * different than the behavior of "test", but is what GRUB does + * (though note as above that we follow symlinks unlike GRUB). */ + result = !S_ISDIR(statbuf.st_mode); break; default: result = false; } - talloc_free(buf); return result; } +/* See comment at builtin_test_op_file for differences between how + * GRUB implements file tests versus Petitboot's GRUB parser. */ static bool builtin_test_op_dir(struct grub2_script *script, char op, const char *dir) { + int rc; + struct stat statbuf; + if (op != 'd') return false; - return parser_check_dir(script->ctx, script->ctx->device, dir) == 0; + rc = parser_stat_path(script->ctx, script->ctx->device, dir, &statbuf); + if (rc) { + return false; + } + + return S_ISDIR(statbuf.st_mode); } static bool builtin_test_op(struct grub2_script *script, diff --git a/discover/parser.c b/discover/parser.c index fbf31b2..5598f96 100644 --- a/discover/parser.c +++ b/discover/parser.c @@ -1,8 +1,6 @@ #include <fcntl.h> #include <stdlib.h> -#include <sys/types.h> -#include <sys/stat.h> #include "types/types.h" #include <file/file.h> @@ -49,24 +47,30 @@ int parser_request_file(struct discover_context *ctx, return rc; } -int parser_check_dir(struct discover_context *ctx, - struct discover_device *dev, const char *dirname) +int parser_stat_path(struct discover_context *ctx, + struct discover_device *dev, const char *path, + struct stat *statbuf) { - struct stat statbuf; - char *path; - int rc; + int rc = -1; + char *full_path; + /* we only support local files at present */ if (!dev->mount_path) return -1; - path = local_path(ctx, dev, dirname); + full_path = local_path(ctx, dev, path); - rc = stat(path, &statbuf); - talloc_free(path); - if (!rc) - return -1; + rc = stat(full_path, statbuf); + if (rc) { + rc = -1; + goto out; + } - return S_ISDIR(statbuf.st_mode) ? 0 : -1; + rc = 0; +out: + talloc_free(full_path); + + return rc; } int parser_replace_file(struct discover_context *ctx, diff --git a/discover/parser.h b/discover/parser.h index e0e8dc6..fc165c5 100644 --- a/discover/parser.h +++ b/discover/parser.h @@ -2,6 +2,9 @@ #define _PARSER_H #include <stdbool.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <unistd.h> #include "device-handler.h" @@ -51,8 +54,9 @@ int parse_user_event(struct discover_context *ctx, struct event *event); /* File IO functions for parsers; these should be the only interface that * parsers use to access a device's filesystem. * - * These are intended for small amounts of data, typically text configuration - * and state files. + * These are intended for small amounts of data, typically text + * configuration and state files. Note that parser_request_file, + * and parser_replace_file work only on non-directories. */ int parser_request_file(struct discover_context *ctx, struct discover_device *dev, const char *filename, @@ -62,7 +66,15 @@ int parser_replace_file(struct discover_context *ctx, char *buf, int len); int parser_request_url(struct discover_context *ctx, struct pb_url *url, char **buf, int *len); -int parser_check_dir(struct discover_context *ctx, - struct discover_device *dev, const char *dirname); +/* parser_stat_path returns 0 if path can be stated on dev by the + * running user. Note that this function follows symlinks, like the + * stat system call. When the function returns 0, also fills in + * statbuf for the path. Returns non-zero on error. This function + * does not have the limitations on file size that the functions above + * do. Unlike some of the functions above, this function also works + * on directories. */ +int parser_stat_path(struct discover_context *ctx, + struct discover_device *dev, const char *path, + struct stat *statbuf); #endif /* _PARSER_H */ diff --git a/test/parser/Makefile.am b/test/parser/Makefile.am index dddb472..e4f9b9c 100644 --- a/test/parser/Makefile.am +++ b/test/parser/Makefile.am @@ -37,6 +37,7 @@ parser_TESTS = \ test/parser/test-grub2-sles-btrfs-snapshot \ test/parser/test-grub2-lexer-error \ test/parser/test-grub2-parser-error \ + test/parser/test-grub2-test-file-ops \ test/parser/test-kboot-single \ test/parser/test-yaboot-empty \ test/parser/test-yaboot-single \ diff --git a/test/parser/parser-test.h b/test/parser/parser-test.h index 21b5b9c..383680f 100644 --- a/test/parser/parser-test.h +++ b/test/parser/parser-test.h @@ -33,10 +33,15 @@ int test_run_parser(struct parser_test *test, const char *parser_name); void test_hotplug_device(struct parser_test *test, struct discover_device *dev); void test_remove_device(struct parser_test *test, struct discover_device *dev); +/* Note that the testing filesystem will only reflect files and + * directories that you explicitly add, so it is possible for a test + * to inconsistently believe that a file exists but that its parent + * directory does not. */ void test_add_file_data(struct parser_test *test, struct discover_device *dev, const char *filename, const void *data, int size); void test_add_dir(struct parser_test *test, struct discover_device *dev, const char *dirname); + void test_set_event_source(struct parser_test *test); void test_set_event_param(struct event *event, const char *name, const char *value); diff --git a/test/parser/test-grub2-test-file-ops.c b/test/parser/test-grub2-test-file-ops.c new file mode 100644 index 0000000..b614fc1 --- /dev/null +++ b/test/parser/test-grub2-test-file-ops.c @@ -0,0 +1,63 @@ + +#include "parser-test.h" + +#if 0 /* PARSER_EMBEDDED_CONFIG */ +status=success + +if [ -f /file_that_does_not_exist -a $status = success ] +then status=fail_f_1 +fi +if [ -f /dir -a $status = success ] +then status=fail_f_2 +fi +if [ ! -f /empty_file -a $status = success ] +then status=fail_f_3 +fi + +if [ -s /file_that_does_not_exist -a $status = success ] +then status=fail_s_1 +fi +if [ ! -s /dir -a $status = success ] +then status=fail_s_2 +fi +if [ -s /empty_file -a $status = success ] +then status=fail_s_3 +fi +if [ ! -s /non_empty_file -a $status = success ] +then status=fail_s_4 +fi + +if [ -d /file_that_does_not_exist -a $status = success ] +then status=fail_d_1 +fi +if [ ! -d /dir -a $status = success ] +then status=fail_d_2 +fi +if [ -d /empty_file -a $status = success ] +then status=fail_d_3 +fi + +menuentry $status { + linux /vmlinux +} +#endif + +void run_test(struct parser_test *test) +{ + struct discover_boot_option *opt; + struct discover_context *ctx; + + ctx = test->ctx; + + test_read_conf_embedded(test, "/grub2/grub.cfg"); + test_add_file_data(test, ctx->device, "/empty_file", "", 0); + test_add_file_data(test, ctx->device, "/non_empty_file", "1", 1); + test_add_dir(test, ctx->device, "/dir"); + + test_run_parser(test, "grub2"); + + check_boot_option_count(ctx, 1); + opt = get_boot_option(ctx, 0); + + check_name(opt, "success"); +} diff --git a/test/parser/utils.c b/test/parser/utils.c index 0050d13..2891969 100644 --- a/test/parser/utils.c +++ b/test/parser/utils.c @@ -193,6 +193,9 @@ void test_add_dir(struct parser_test *test, struct discover_device *dev, file->type = TEST_DIR; file->dev = dev; file->name = dirname; + /* Pick a non-zero size for directories so that "[ -s <dir + * path> ]" sees that the file has non-zero size. */ + file->size = 1; list_add(&test->files, &file->list); } @@ -241,20 +244,34 @@ int parser_request_file(struct discover_context *ctx, return -1; } -int parser_check_dir(struct discover_context *ctx, - struct discover_device *dev, const char *dirname) +int parser_stat_path(struct discover_context *ctx, + struct discover_device *dev, const char *path, + struct stat *statbuf) { struct parser_test *test = ctx->test_data; struct test_file *file; - printf("%s: %s\n", __func__, dirname); - list_for_each_entry(&test->files, file, list) { if (file->dev != dev) continue; - if (strcmp(file->name, dirname)) + if (strcmp(file->name, path)) continue; - return file->type == TEST_DIR ? 0 : -1; + + statbuf->st_size = (off_t)file->size; + switch (file->type) { + case TEST_FILE: + statbuf->st_mode = S_IFREG; + break; + case TEST_DIR: + statbuf->st_mode = S_IFDIR; + break; + default: + fprintf(stderr, "%s: bad test file mode %d!", __func__, + file->type); + exit(EXIT_FAILURE); + } + + return 0; } return -1; |