[U-Boot] [PATCH V3 01/12] fs: fix generic save command implementation
From: Stephen Warren swarren@nvidia.com
Fix a few issues with the generic "save" shell command, and fs_write() function.
1) fstypes[].write wasn't filled in for some file-systems, and isn't checked when used, which could cause crashes/... if executing save on e.g. fat/ext filesystems.
2) fs_write() requires the length argument to be non-zero, since it needs to know exactly how many bytes to write. Adjust the comments and code according to this.
3) fs_write() wasn't prototyped in <fs.h> like other generic functions; other code should be able to call this directly rather than invoking the "save" shell command.
Signed-off-by: Stephen Warren swarren@nvidia.com Acked-by: Simon Glass sjg@chromium.org --- v3: No change. v2: No change. --- fs/fs.c | 9 +++------ include/fs.h | 10 ++++++++++ 2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/fs/fs.c b/fs/fs.c index be1855d1291f..9c2ef6b6597c 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -75,6 +75,7 @@ static struct fstype_info fstypes[] = { .close = fat_close, .ls = file_fat_ls, .read = fat_read_file, + .write = fs_write_unsupported, }, #endif #ifdef CONFIG_FS_EXT4 @@ -84,6 +85,7 @@ static struct fstype_info fstypes[] = { .close = ext4fs_close, .ls = ext4fs_ls, .read = ext4_read_file, + .write = fs_write_unsupported, }, #endif #ifdef CONFIG_SANDBOX @@ -212,16 +214,11 @@ int fs_write(const char *filename, ulong addr, int offset, int len) void *buf; int ret;
- /* - * We don't actually know how many bytes are being read, since len==0 - * means read the whole file. - */ buf = map_sysmem(addr, len); ret = info->write(filename, buf, offset, len); unmap_sysmem(buf);
- /* If we requested a specific number of bytes, check we got it */ - if (ret >= 0 && len && ret != len) { + if (ret >= 0 && ret != len) { printf("** Unable to write file %s **\n", filename); ret = -1; } diff --git a/include/fs.h b/include/fs.h index 7d9403ed8758..97b0094e954b 100644 --- a/include/fs.h +++ b/include/fs.h @@ -55,6 +55,16 @@ int fs_ls(const char *dirname); int fs_read(const char *filename, ulong addr, int offset, int len);
/* + * Write file "filename" to the partition previously set by fs_set_blk_dev(), + * from address "addr", starting at byte offset "offset", and writing "len" + * bytes. "offset" may be 0 to write to the start of the file. Note that not + * all filesystem types support offset!=0. + * + * Returns number of bytes read on success. Returns <= 0 on error. + */ +int fs_write(const char *filename, ulong addr, int offset, int len); + +/* * Common implementation for various filesystem commands, optionally limited * to a specific filesystem type via the fstype parameter. */
From: Stephen Warren swarren@nvidia.com
This could be used in scripts such as:
if test -e mmc 0:1 /boot/boot.scr; then load mmc 0:1 ${scriptaddr} /boot/boot.scr source ${scriptaddr} fi
rather than:
if load mmc 0:1 ${scriptaddr} /boot/boot.scr; then source ${scriptaddr} fi
This prevents errors being printed by attempts to load non-existent files, which can be important when checking for a large set of files, such as /boot/boot.scr.uimg, /boot/boot.scr, /boot/extlinux.conf, /boot.scr.uimg, /boot.scr, /extlinux.conf.
Signed-off-by: Stephen Warren swarren@nvidia.com Acked-by: Simon Glass sjg@chromium.org --- v3: * Remove addition of "exists" command; it's implemented in a separate patch later, as an operator in the "test" command. * Invert return value of fs_exists()/file_exists() so it returns a logical value; the mapping to shell command return value is performed (later) in the test command. v2: No change. --- fs/fs.c | 32 ++++++++++++++++++++++++++++++++ include/fs.h | 9 +++++++++ 2 files changed, 41 insertions(+)
diff --git a/fs/fs.c b/fs/fs.c index 9c2ef6b6597c..8fe2403a46ae 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -41,6 +41,11 @@ static inline int fs_ls_unsupported(const char *dirname) return -1; }
+static inline int fs_exists_unsupported(const char *filename) +{ + return 0; +} + static inline int fs_read_unsupported(const char *filename, void *buf, int offset, int len) { @@ -62,6 +67,7 @@ struct fstype_info { int (*probe)(block_dev_desc_t *fs_dev_desc, disk_partition_t *fs_partition); int (*ls)(const char *dirname); + int (*exists)(const char *filename); int (*read)(const char *filename, void *buf, int offset, int len); int (*write)(const char *filename, void *buf, int offset, int len); void (*close)(void); @@ -74,6 +80,7 @@ static struct fstype_info fstypes[] = { .probe = fat_set_blk_dev, .close = fat_close, .ls = file_fat_ls, + .exists = fs_exists_unsupported, .read = fat_read_file, .write = fs_write_unsupported, }, @@ -84,6 +91,7 @@ static struct fstype_info fstypes[] = { .probe = ext4fs_probe, .close = ext4fs_close, .ls = ext4fs_ls, + .exists = fs_exists_unsupported, .read = ext4_read_file, .write = fs_write_unsupported, }, @@ -94,6 +102,7 @@ static struct fstype_info fstypes[] = { .probe = sandbox_fs_set_blk_dev, .close = sandbox_fs_close, .ls = sandbox_fs_ls, + .exists = fs_exists_unsupported, .read = fs_read_sandbox, .write = fs_write_sandbox, }, @@ -103,6 +112,7 @@ static struct fstype_info fstypes[] = { .probe = fs_probe_unsupported, .close = fs_close_unsupported, .ls = fs_ls_unsupported, + .exists = fs_exists_unsupported, .read = fs_read_unsupported, .write = fs_write_unsupported, }, @@ -184,6 +194,19 @@ int fs_ls(const char *dirname) return ret; }
+int fs_exists(const char *filename) +{ + int ret; + + struct fstype_info *info = fs_get_info(fs_type); + + ret = info->exists(filename); + + fs_close(); + + return ret; +} + int fs_read(const char *filename, ulong addr, int offset, int len) { struct fstype_info *info = fs_get_info(fs_type); @@ -309,6 +332,15 @@ int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], return 0; }
+int file_exists(const char *dev_type, const char *dev_part, const char *file, + int fstype) +{ + if (fs_set_blk_dev(dev_type, dev_part, fstype)) + return 0; + + return fs_exists(file); +} + int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], int fstype) { diff --git a/include/fs.h b/include/fs.h index 97b0094e954b..26de0539f7d9 100644 --- a/include/fs.h +++ b/include/fs.h @@ -44,6 +44,13 @@ int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype); int fs_ls(const char *dirname);
/* + * Determine whether a file exists + * + * Returns 1 if the file exists, 0 if it doesn't exist. + */ +int fs_exists(const char *filename); + +/* * Read file "filename" from the partition previously set by fs_set_blk_dev(), * to address "addr", starting at byte offset "offset", and reading "len" * bytes. "offset" may be 0 to read from the start of the file. "len" may be @@ -72,6 +79,8 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], int fstype); int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], int fstype); +int file_exists(const char *dev_type, const char *dev_part, const char *file, + int fstype); int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], int fstype);
On 27 January 2014 13:49, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
This could be used in scripts such as:
if test -e mmc 0:1 /boot/boot.scr; then load mmc 0:1 ${scriptaddr} /boot/boot.scr source ${scriptaddr} fi
rather than:
if load mmc 0:1 ${scriptaddr} /boot/boot.scr; then source ${scriptaddr} fi
This prevents errors being printed by attempts to load non-existent files, which can be important when checking for a large set of files, such as /boot/boot.scr.uimg, /boot/boot.scr, /boot/extlinux.conf, /boot.scr.uimg, /boot.scr, /extlinux.conf.
Signed-off-by: Stephen Warren swarren@nvidia.com Acked-by: Simon Glass sjg@chromium.org
nit in case you re-issue the series, infrastructure is one word.
Regards, Simon
From: Stephen Warren swarren@nvidia.com
FAT and ext4 expect that the passed in block device descriptor not be NULL. This causes problems on sandbox, where get_device_and_partition() succeeds for the "host" device, yet passes back a NULL device descriptor. Add special handling for this situation, so that the generic filesystem commands operate as expected on sandbox.
Signed-off-by: Stephen Warren swarren@nvidia.com --- v3: New patch. --- fs/fs.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/fs/fs.c b/fs/fs.c index 8fe2403a46ae..653e597c3cc9 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -64,6 +64,7 @@ static inline void fs_close_unsupported(void)
struct fstype_info { int fstype; + bool null_dev_desc_ok; int (*probe)(block_dev_desc_t *fs_dev_desc, disk_partition_t *fs_partition); int (*ls)(const char *dirname); @@ -77,6 +78,7 @@ static struct fstype_info fstypes[] = { #ifdef CONFIG_FS_FAT { .fstype = FS_TYPE_FAT, + .null_dev_desc_ok = false, .probe = fat_set_blk_dev, .close = fat_close, .ls = file_fat_ls, @@ -88,6 +90,7 @@ static struct fstype_info fstypes[] = { #ifdef CONFIG_FS_EXT4 { .fstype = FS_TYPE_EXT, + .null_dev_desc_ok = false, .probe = ext4fs_probe, .close = ext4fs_close, .ls = ext4fs_ls, @@ -99,6 +102,7 @@ static struct fstype_info fstypes[] = { #ifdef CONFIG_SANDBOX { .fstype = FS_TYPE_SANDBOX, + .null_dev_desc_ok = true, .probe = sandbox_fs_set_blk_dev, .close = sandbox_fs_close, .ls = sandbox_fs_ls, @@ -109,6 +113,7 @@ static struct fstype_info fstypes[] = { #endif { .fstype = FS_TYPE_ANY, + .null_dev_desc_ok = true, .probe = fs_probe_unsupported, .close = fs_close_unsupported, .ls = fs_ls_unsupported, @@ -162,6 +167,9 @@ int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype) fstype != info->fstype) continue;
+ if (!fs_dev_desc && !info->null_dev_desc_ok) + continue; + if (!info->probe(fs_dev_desc, &fs_partition)) { fs_type = info->fstype; return 0;
Hi Stephen,
On 27 January 2014 13:49, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
FAT and ext4 expect that the passed in block device descriptor not be NULL. This causes problems on sandbox, where get_device_and_partition() succeeds for the "host" device, yet passes back a NULL device descriptor. Add special handling for this situation, so that the generic filesystem commands operate as expected on sandbox.
Signed-off-by: Stephen Warren swarren@nvidia.com
v3: New patch.
fs/fs.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/fs/fs.c b/fs/fs.c index 8fe2403a46ae..653e597c3cc9 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -64,6 +64,7 @@ static inline void fs_close_unsupported(void)
struct fstype_info { int fstype;
bool null_dev_desc_ok;
I suggest adding a comment for this field.
int (*probe)(block_dev_desc_t *fs_dev_desc, disk_partition_t *fs_partition); int (*ls)(const char *dirname);@@ -77,6 +78,7 @@ static struct fstype_info fstypes[] = { #ifdef CONFIG_FS_FAT { .fstype = FS_TYPE_FAT,
.null_dev_desc_ok = false, .probe = fat_set_blk_dev, .close = fat_close, .ls = file_fat_ls,@@ -88,6 +90,7 @@ static struct fstype_info fstypes[] = { #ifdef CONFIG_FS_EXT4 { .fstype = FS_TYPE_EXT,
.null_dev_desc_ok = false, .probe = ext4fs_probe, .close = ext4fs_close, .ls = ext4fs_ls,@@ -99,6 +102,7 @@ static struct fstype_info fstypes[] = { #ifdef CONFIG_SANDBOX { .fstype = FS_TYPE_SANDBOX,
.null_dev_desc_ok = true, .probe = sandbox_fs_set_blk_dev, .close = sandbox_fs_close, .ls = sandbox_fs_ls,@@ -109,6 +113,7 @@ static struct fstype_info fstypes[] = { #endif { .fstype = FS_TYPE_ANY,
.null_dev_desc_ok = true, .probe = fs_probe_unsupported, .close = fs_close_unsupported, .ls = fs_ls_unsupported,@@ -162,6 +167,9 @@ int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype) fstype != info->fstype) continue;
if (!fs_dev_desc && !info->null_dev_desc_ok)continue;if (!info->probe(fs_dev_desc, &fs_partition)) { fs_type = info->fstype; return 0;-- 1.8.1.5
Regards, Simon
From: Stephen Warren swarren@nvidia.com
do_test() currently uses strcmp() twice to determine which operator is present; once to determine how many arguments the operator needs, then a second time to actually decode the operator and implement it.
Rewrite the code so that a table lookup is used to translate the operator string to an integer, and use a more efficient switch statement to decode and execute the operator.
This approach also acts as enablement for the following patches.
This patch should introduce no behavioural change.
Signed-off-by: Stephen Warren swarren@nvidia.com --- v3: New patch. --- common/cmd_test.c | 177 +++++++++++++++++++++++++++++++++--------------------- 1 file changed, 110 insertions(+), 67 deletions(-)
diff --git a/common/cmd_test.c b/common/cmd_test.c index bacc3684069b..69b1b4cee69a 100644 --- a/common/cmd_test.c +++ b/common/cmd_test.c @@ -17,10 +17,48 @@ #include <common.h> #include <command.h>
+#define OP_INVALID 0 +#define OP_OR 2 +#define OP_AND 3 +#define OP_STR_EMPTY 4 +#define OP_STR_NEMPTY 5 +#define OP_STR_EQ 6 +#define OP_STR_NEQ 7 +#define OP_STR_LT 8 +#define OP_STR_GT 9 +#define OP_INT_EQ 10 +#define OP_INT_NEQ 11 +#define OP_INT_LT 12 +#define OP_INT_LE 13 +#define OP_INT_GT 14 +#define OP_INT_GE 15 + +const struct { + int arg; + const char *str; + int op; + int adv; +} op_adv[] = { + {0, "-o", OP_OR, 1}, + {0, "-a", OP_AND, 1}, + {0, "-z", OP_STR_EMPTY, 2}, + {0, "-n", OP_STR_NEMPTY, 2}, + {1, "=", OP_STR_EQ, 3}, + {1, "!=", OP_STR_NEQ, 3}, + {1, "<", OP_STR_LT, 3}, + {1, ">", OP_STR_GT, 3}, + {1, "-eq", OP_INT_EQ, 3}, + {1, "-ne", OP_INT_NEQ, 3}, + {1, "-lt", OP_INT_LT, 3}, + {1, "-le", OP_INT_LE, 3}, + {1, "-gt", OP_INT_GT, 3}, + {1, "-ge", OP_INT_GE, 3}, +}; + static int do_test(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { char * const *ap; - int left, adv, expr, last_expr, neg, last_cmp; + int i, op, left, adv, expr, last_expr, neg, last_cmp;
/* args? */ if (argc < 3) @@ -45,83 +83,88 @@ static int do_test(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) neg = 0;
expr = -1; - last_cmp = -1; + last_cmp = OP_INVALID; last_expr = -1; while (left > 0) { - - if (strcmp(ap[0], "-o") == 0 || strcmp(ap[0], "-a") == 0) - adv = 1; - else if (strcmp(ap[0], "-z") == 0 || strcmp(ap[0], "-n") == 0) - adv = 2; - else - adv = 3; - - if (left < adv) { + for (i = 0; i < ARRAY_SIZE(op_adv); i++) { + if (left <= op_adv[i].arg) + continue; + if (!strcmp(ap[op_adv[i].arg], op_adv[i].str)) { + op = op_adv[i].op; + adv = op_adv[i].adv; + break; + } + } + if (i == ARRAY_SIZE(op_adv)) { expr = 1; break; } - - if (adv == 1) { - if (strcmp(ap[0], "-o") == 0) { - last_expr = expr; - last_cmp = 0; - } else if (strcmp(ap[0], "-a") == 0) { - last_expr = expr; - last_cmp = 1; - } else { - expr = 1; - break; - } + if (left < adv) { + expr = 1; + break; }
- if (adv == 2) { - if (strcmp(ap[0], "-z") == 0) - expr = strlen(ap[1]) == 0 ? 1 : 0; - else if (strcmp(ap[0], "-n") == 0) - expr = strlen(ap[1]) == 0 ? 0 : 1; - else { - expr = 1; - break; - } - - if (last_cmp == 0) - expr = last_expr || expr; - else if (last_cmp == 1) - expr = last_expr && expr; - last_cmp = -1; + switch (op) { + case OP_STR_EMPTY: + expr = strlen(ap[1]) == 0 ? 1 : 0; + break; + case OP_STR_NEMPTY: + expr = strlen(ap[1]) == 0 ? 0 : 1; + break; + case OP_STR_EQ: + expr = strcmp(ap[0], ap[2]) == 0; + break; + case OP_STR_NEQ: + expr = strcmp(ap[0], ap[2]) != 0; + break; + case OP_STR_LT: + expr = strcmp(ap[0], ap[2]) < 0; + break; + case OP_STR_GT: + expr = strcmp(ap[0], ap[2]) > 0; + break; + case OP_INT_EQ: + expr = simple_strtol(ap[0], NULL, 10) == + simple_strtol(ap[2], NULL, 10); + break; + case OP_INT_NEQ: + expr = simple_strtol(ap[0], NULL, 10) != + simple_strtol(ap[2], NULL, 10); + break; + case OP_INT_LT: + expr = simple_strtol(ap[0], NULL, 10) < + simple_strtol(ap[2], NULL, 10); + break; + case OP_INT_LE: + expr = simple_strtol(ap[0], NULL, 10) <= + simple_strtol(ap[2], NULL, 10); + break; + case OP_INT_GT: + expr = simple_strtol(ap[0], NULL, 10) > + simple_strtol(ap[2], NULL, 10); + break; + case OP_INT_GE: + expr = simple_strtol(ap[0], NULL, 10) >= + simple_strtol(ap[2], NULL, 10); + break; }
- if (adv == 3) { - if (strcmp(ap[1], "=") == 0) - expr = strcmp(ap[0], ap[2]) == 0; - else if (strcmp(ap[1], "!=") == 0) - expr = strcmp(ap[0], ap[2]) != 0; - else if (strcmp(ap[1], ">") == 0) - expr = strcmp(ap[0], ap[2]) > 0; - else if (strcmp(ap[1], "<") == 0) - expr = strcmp(ap[0], ap[2]) < 0; - else if (strcmp(ap[1], "-eq") == 0) - expr = simple_strtol(ap[0], NULL, 10) == simple_strtol(ap[2], NULL, 10); - else if (strcmp(ap[1], "-ne") == 0) - expr = simple_strtol(ap[0], NULL, 10) != simple_strtol(ap[2], NULL, 10); - else if (strcmp(ap[1], "-lt") == 0) - expr = simple_strtol(ap[0], NULL, 10) < simple_strtol(ap[2], NULL, 10); - else if (strcmp(ap[1], "-le") == 0) - expr = simple_strtol(ap[0], NULL, 10) <= simple_strtol(ap[2], NULL, 10); - else if (strcmp(ap[1], "-gt") == 0) - expr = simple_strtol(ap[0], NULL, 10) > simple_strtol(ap[2], NULL, 10); - else if (strcmp(ap[1], "-ge") == 0) - expr = simple_strtol(ap[0], NULL, 10) >= simple_strtol(ap[2], NULL, 10); - else { - expr = 1; - break; - } - - if (last_cmp == 0) + switch (op) { + case OP_OR: + last_expr = expr; + last_cmp = OP_OR; + break; + case OP_AND: + last_expr = expr; + last_cmp = OP_AND; + break; + default: + if (last_cmp == OP_OR) expr = last_expr || expr; - else if (last_cmp == 1) + else if (last_cmp == OP_AND) expr = last_expr && expr; - last_cmp = -1; + last_cmp = OP_INVALID; + break; }
ap += adv; left -= adv;
Hi Stephen,
On 27 January 2014 13:49, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
do_test() currently uses strcmp() twice to determine which operator is present; once to determine how many arguments the operator needs, then a second time to actually decode the operator and implement it.
Rewrite the code so that a table lookup is used to translate the operator string to an integer, and use a more efficient switch statement to decode and execute the operator.
This approach also acts as enablement for the following patches.
This patch should introduce no behavioural change.
Signed-off-by: Stephen Warren swarren@nvidia.com
v3: New patch.
Wow that's some interesting code...it took me a while to understand both the old and the new code. It looks correct to me but I wonder if it is deserving of some tests? Something like test/command_ut.c might show a simple way to run some tests.
Regards, Simon
On 01/31/2014 05:03 PM, Simon Glass wrote:
Hi Stephen,
On 27 January 2014 13:49, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
do_test() currently uses strcmp() twice to determine which operator is present; once to determine how many arguments the operator needs, then a second time to actually decode the operator and implement it.
Rewrite the code so that a table lookup is used to translate the operator string to an integer, and use a more efficient switch statement to decode and execute the operator.
This approach also acts as enablement for the following patches.
This patch should introduce no behavioural change.
Signed-off-by: Stephen Warren swarren@nvidia.com
v3: New patch.
Wow that's some interesting code...it took me a while to understand both the old and the new code. It looks correct to me but I wonder if it is deserving of some tests? Something like test/command_ut.c might show a simple way to run some tests.
OK, I'll send V4 of this series with your minor issues addressed, and I'll send a separate follow-on series which adds the unit tests, just so it doesn't delay or cause revisions to the main series.
Hi Stephen,
On 3 February 2014 13:19, Stephen Warren swarren@wwwdotorg.org wrote:
On 01/31/2014 05:03 PM, Simon Glass wrote:
Hi Stephen,
On 27 January 2014 13:49, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
do_test() currently uses strcmp() twice to determine which operator is present; once to determine how many arguments the operator needs, then a second time to actually decode the operator and implement it.
Rewrite the code so that a table lookup is used to translate the operator string to an integer, and use a more efficient switch statement to decode and execute the operator.
This approach also acts as enablement for the following patches.
This patch should introduce no behavioural change.
Signed-off-by: Stephen Warren swarren@nvidia.com
v3: New patch.
Wow that's some interesting code...it took me a while to understand both the old and the new code. It looks correct to me but I wonder if it is deserving of some tests? Something like test/command_ut.c might show a simple way to run some tests.
OK, I'll send V4 of this series with your minor issues addressed, and I'll send a separate follow-on series which adds the unit tests, just so it doesn't delay or cause revisions to the main series.
Sounds good, thanks.
Regards, Simon
From: Stephen Warren swarren@nvidia.com
This better mirrors the behaviour of bash, for example:
$ if test -z = -z; then echo yes; else echo no; fi yes
This is parsed as a string comparison of "-z" and "-z", since the check for the binary "=" operator occurs first. Without this change, the command would be parsed as a -z test of "-", followed by a syntax error; a trailing -z without and operand.
This is a behavioural change, but I believe any commands affected were previously invalid or bizarely formed.
Signed-off-by: Stephen Warren swarren@nvidia.com --- v3: New patch. --- common/cmd_test.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/common/cmd_test.c b/common/cmd_test.c index 69b1b4cee69a..e65dd531877e 100644 --- a/common/cmd_test.c +++ b/common/cmd_test.c @@ -39,10 +39,6 @@ const struct { int op; int adv; } op_adv[] = { - {0, "-o", OP_OR, 1}, - {0, "-a", OP_AND, 1}, - {0, "-z", OP_STR_EMPTY, 2}, - {0, "-n", OP_STR_NEMPTY, 2}, {1, "=", OP_STR_EQ, 3}, {1, "!=", OP_STR_NEQ, 3}, {1, "<", OP_STR_LT, 3}, @@ -53,6 +49,10 @@ const struct { {1, "-le", OP_INT_LE, 3}, {1, "-gt", OP_INT_GT, 3}, {1, "-ge", OP_INT_GE, 3}, + {0, "-o", OP_OR, 1}, + {0, "-a", OP_AND, 1}, + {0, "-z", OP_STR_EMPTY, 2}, + {0, "-n", OP_STR_NEMPTY, 2}, };
static int do_test(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
From: Stephen Warren swarren@nvidia.com
Currently, ! can only be parsed as the first operator in an expression. This prevents the following from working:
$ if test ! ! 1 -eq 1; then echo yes; else echo no; fi yes $ if test ! 1 -eq 2 -a ! 3 -eq 4; then echo yes; else echo no; fi yes
Fix this by parsing ! like any other operator, and and handling it similarly to -a and -o.
Signed-off-by: Stephen Warren swarren@nvidia.com --- v3: New patch. --- common/cmd_test.c | 43 ++++++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 19 deletions(-)
diff --git a/common/cmd_test.c b/common/cmd_test.c index e65dd531877e..b927d09eb3e0 100644 --- a/common/cmd_test.c +++ b/common/cmd_test.c @@ -18,6 +18,7 @@ #include <command.h>
#define OP_INVALID 0 +#define OP_NOT 1 #define OP_OR 2 #define OP_AND 3 #define OP_STR_EMPTY 4 @@ -49,6 +50,7 @@ const struct { {1, "-le", OP_INT_LE, 3}, {1, "-gt", OP_INT_GT, 3}, {1, "-ge", OP_INT_GE, 3}, + {0, "!", OP_NOT, 1}, {0, "-o", OP_OR, 1}, {0, "-a", OP_AND, 1}, {0, "-z", OP_STR_EMPTY, 2}, @@ -58,7 +60,7 @@ const struct { static int do_test(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { char * const *ap; - int i, op, left, adv, expr, last_expr, neg, last_cmp; + int i, op, left, adv, expr, last_expr, last_unop, last_binop;
/* args? */ if (argc < 3) @@ -73,17 +75,11 @@ static int do_test(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } #endif
- last_expr = 0; - left = argc - 1; ap = argv + 1; - if (left > 0 && strcmp(ap[0], "!") == 0) { - neg = 1; - ap++; - left--; - } else - neg = 0; - + left = argc - 1; + ap = argv + 1; expr = -1; - last_cmp = OP_INVALID; + last_unop = OP_INVALID; + last_binop = OP_INVALID; last_expr = -1; while (left > 0) { for (i = 0; i < ARRAY_SIZE(op_adv); i++) { @@ -152,27 +148,36 @@ static int do_test(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) switch (op) { case OP_OR: last_expr = expr; - last_cmp = OP_OR; + last_binop = OP_OR; break; case OP_AND: last_expr = expr; - last_cmp = OP_AND; + last_binop = OP_AND; + break; + case OP_NOT: + if (last_unop == OP_NOT) + last_unop = OP_INVALID; + else + last_unop = OP_NOT; break; default: - if (last_cmp == OP_OR) + if (last_unop == OP_NOT) { + expr = !expr; + last_unop = OP_INVALID; + } + + if (last_binop == OP_OR) expr = last_expr || expr; - else if (last_cmp == OP_AND) + else if (last_binop == OP_AND) expr = last_expr && expr; - last_cmp = OP_INVALID; + last_binop = OP_INVALID; + break; }
ap += adv; left -= adv; }
- if (neg) - expr = !expr; - expr = !expr;
debug (": returns %d\n", expr);
From: Stephen Warren swarren@nvidia.com
This emulates bash: $ if test; then echo yes; else echo no; fi no
Currently, the code sets expr = -1 in this case, which gets mapped to 0 (true) at the end of do_test() by the logical -> shell exit code conversion.
Signed-off-by: Stephen Warren swarren@nvidia.com --- v3: New patch. --- common/cmd_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/cmd_test.c b/common/cmd_test.c index b927d09eb3e0..4c2f967c6dc0 100644 --- a/common/cmd_test.c +++ b/common/cmd_test.c @@ -77,7 +77,7 @@ static int do_test(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
left = argc - 1; ap = argv + 1; - expr = -1; + expr = 0; last_unop = OP_INVALID; last_binop = OP_INVALID; last_expr = -1;
From: Stephen Warren swarren@nvidia.com
This is much like a regular shell's -e operator, except that it takes multiple arguments to specify the device type and device/partition ID in addition to the usual filename:
if test -e mmc 0:1 /boot/boot.scr; then echo yes; else echo no; fi
Signed-off-by: Stephen Warren swarren@nvidia.com --- v3: New patch. --- common/cmd_test.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/common/cmd_test.c b/common/cmd_test.c index 4c2f967c6dc0..c93fe7823100 100644 --- a/common/cmd_test.c +++ b/common/cmd_test.c @@ -16,6 +16,7 @@
#include <common.h> #include <command.h> +#include <fs.h>
#define OP_INVALID 0 #define OP_NOT 1 @@ -33,6 +34,7 @@ #define OP_INT_LE 13 #define OP_INT_GT 14 #define OP_INT_GE 15 +#define OP_FILE_EXISTS 16
const struct { int arg; @@ -55,6 +57,7 @@ const struct { {0, "-a", OP_AND, 1}, {0, "-z", OP_STR_EMPTY, 2}, {0, "-n", OP_STR_NEMPTY, 2}, + {0, "-e", OP_FILE_EXISTS, 4}, };
static int do_test(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) @@ -143,6 +146,9 @@ static int do_test(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) expr = simple_strtol(ap[0], NULL, 10) >= simple_strtol(ap[2], NULL, 10); break; + case OP_FILE_EXISTS: + expr = file_exists(ap[1], ap[2], ap[3], FS_TYPE_ANY); + break; }
switch (op) {
Hi Stephen,
On 27 January 2014 13:49, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
This is much like a regular shell's -e operator, except that it takes multiple arguments to specify the device type and device/partition ID in addition to the usual filename:
if test -e mmc 0:1 /boot/boot.scr; then echo yes; else echo no; fi
Signed-off-by: Stephen Warren swarren@nvidia.com
That's a really clever implementation.
Regards, Simon
From: Stephen Warren swarren@nvidia.com
This hooks into the generic "file exists" support added in an earlier patch, and provides an implementation for the sandbox test environment.
Signed-off-by: Stephen Warren swarren@nvidia.com Acked-by: Simon Glass sjg@chromium.org --- v3: * Remove "sb exists" command; it's part of "test" now. * Invert return value of exists(), per change to previous patch. v2: No change. --- fs/fs.c | 2 +- fs/sandbox/sandboxfs.c | 8 ++++++++ include/sandboxfs.h | 1 + 3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/fs/fs.c b/fs/fs.c index 653e597c3cc9..81ff1959fae5 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -106,7 +106,7 @@ static struct fstype_info fstypes[] = { .probe = sandbox_fs_set_blk_dev, .close = sandbox_fs_close, .ls = sandbox_fs_ls, - .exists = fs_exists_unsupported, + .exists = sandbox_fs_exists, .read = fs_read_sandbox, .write = fs_write_sandbox, }, diff --git a/fs/sandbox/sandboxfs.c b/fs/sandbox/sandboxfs.c index dd028da8e32b..85079788c990 100644 --- a/fs/sandbox/sandboxfs.c +++ b/fs/sandbox/sandboxfs.c @@ -72,6 +72,14 @@ int sandbox_fs_ls(const char *dirname) return 0; }
+int sandbox_fs_exists(const char *filename) +{ + ssize_t sz; + + sz = os_get_filesize(filename); + return sz >= 0; +} + void sandbox_fs_close(void) { } diff --git a/include/sandboxfs.h b/include/sandboxfs.h index 8ea8cb7e2e62..a51ad13044e1 100644 --- a/include/sandboxfs.h +++ b/include/sandboxfs.h @@ -25,6 +25,7 @@ long sandbox_fs_read_at(const char *filename, unsigned long pos,
void sandbox_fs_close(void); int sandbox_fs_ls(const char *dirname); +int sandbox_fs_exists(const char *filename); int fs_read_sandbox(const char *filename, void *buf, int offset, int len); int fs_write_sandbox(const char *filename, void *buf, int offset, int len);
From: Stephen Warren swarren@nvidia.com
Since the generic ls command no longer segfaults sandbox, enabling it.
Signed-off-by: Stephen Warren swarren@nvidia.com --- v3: New patch. --- include/configs/sandbox.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h index a6d55822b82e..e77d06bcd3ed 100644 --- a/include/configs/sandbox.h +++ b/include/configs/sandbox.h @@ -42,6 +42,7 @@ #define CONFIG_CMD_PART #define CONFIG_DOS_PARTITION #define CONFIG_HOST_MAX_DEVICES 4 +#define CONFIG_CMD_FS_GENERIC
#define CONFIG_SYS_VSNPRINTF
On 27 January 2014 13:50, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
Since the generic ls command no longer segfaults sandbox, enabling it.
Signed-off-by: Stephen Warren swarren@nvidia.com
Acked-by: Simon Glass sjg@chromium.org
v3: New patch.
include/configs/sandbox.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h index a6d55822b82e..e77d06bcd3ed 100644 --- a/include/configs/sandbox.h +++ b/include/configs/sandbox.h @@ -42,6 +42,7 @@ #define CONFIG_CMD_PART #define CONFIG_DOS_PARTITION #define CONFIG_HOST_MAX_DEVICES 4 +#define CONFIG_CMD_FS_GENERIC
#define CONFIG_SYS_VSNPRINTF
-- 1.8.1.5
From: Stephen Warren swarren@nvidia.com
This hooks into the generic "file exists" support added in an earlier patch, and provides an implementation for the ext4 filesystem.
Signed-off-by: Stephen Warren swarren@nvidia.com Acked-by: Simon Glass sjg@chromium.org --- v3: Invert return value of exists(), per change to previous patch. v2: No change. --- fs/ext4/ext4fs.c | 8 ++++++++ fs/fs.c | 2 +- include/ext4fs.h | 1 + 3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c index 735b2564175b..417ce7b63bf0 100644 --- a/fs/ext4/ext4fs.c +++ b/fs/ext4/ext4fs.c @@ -174,6 +174,14 @@ int ext4fs_ls(const char *dirname) return 0; }
+int ext4fs_exists(const char *filename) +{ + int file_len; + + file_len = ext4fs_open(filename); + return file_len >= 0; +} + int ext4fs_read(char *buf, unsigned len) { if (ext4fs_root == NULL || ext4fs_file == NULL) diff --git a/fs/fs.c b/fs/fs.c index 81ff1959fae5..e1e4fcc236d6 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -94,7 +94,7 @@ static struct fstype_info fstypes[] = { .probe = ext4fs_probe, .close = ext4fs_close, .ls = ext4fs_ls, - .exists = fs_exists_unsupported, + .exists = ext4fs_exists, .read = ext4_read_file, .write = fs_write_unsupported, }, diff --git a/include/ext4fs.h b/include/ext4fs.h index 242938039662..aacb147de24b 100644 --- a/include/ext4fs.h +++ b/include/ext4fs.h @@ -134,6 +134,7 @@ int ext4fs_read(char *buf, unsigned len); int ext4fs_mount(unsigned part_length); void ext4fs_close(void); int ext4fs_ls(const char *dirname); +int ext4fs_exists(const char *filename); void ext4fs_free_node(struct ext2fs_node *node, struct ext2fs_node *currroot); int ext4fs_devread(lbaint_t sector, int byte_offset, int byte_len, char *buf); void ext4fs_set_blk_dev(block_dev_desc_t *rbdd, disk_partition_t *info);
From: Stephen Warren swarren@nvidia.com
This hooks into the generic "file exists" support added in an earlier patch, and provides an implementation for the FAT filesystem.
Signed-off-by: Stephen Warren swarren@nvidia.com --- v3: * s/ext/fat/ in the commit description too:-( * Invert return value of exists(), per change to previous patch. v2: s/ext/fat/ in the commit subject. --- fs/fat/fat.c | 18 ++++++++++++++---- fs/fs.c | 2 +- include/fat.h | 1 + 3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index b41d62e3c386..54f42eae0d05 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -808,7 +808,7 @@ __u8 do_fat_read_at_block[MAX_CLUSTSIZE]
long do_fat_read_at(const char *filename, unsigned long pos, void *buffer, - unsigned long maxsize, int dols) + unsigned long maxsize, int dols, int dogetsize) { char fnamecopy[2048]; boot_sector bs; @@ -1152,7 +1152,10 @@ rootdir_done: subname = nextname; }
- ret = get_contents(mydata, dentptr, pos, buffer, maxsize); + if (dogetsize) + ret = FAT2CPU32(dentptr->size); + else + ret = get_contents(mydata, dentptr, pos, buffer, maxsize); debug("Size: %d, got: %ld\n", FAT2CPU32(dentptr->size), ret);
exit: @@ -1163,7 +1166,7 @@ exit: long do_fat_read(const char *filename, void *buffer, unsigned long maxsize, int dols) { - return do_fat_read_at(filename, 0, buffer, maxsize, dols); + return do_fat_read_at(filename, 0, buffer, maxsize, dols, 0); }
int file_fat_detectfs(void) @@ -1233,11 +1236,18 @@ int file_fat_ls(const char *dir) return do_fat_read(dir, NULL, 0, LS_YES); }
+int fat_exists(const char *filename) +{ + int sz; + sz = do_fat_read_at(filename, 0, NULL, 0, LS_NO, 1); + return sz >= 0; +} + long file_fat_read_at(const char *filename, unsigned long pos, void *buffer, unsigned long maxsize) { printf("reading %s\n", filename); - return do_fat_read_at(filename, pos, buffer, maxsize, LS_NO); + return do_fat_read_at(filename, pos, buffer, maxsize, LS_NO, 0); }
long file_fat_read(const char *filename, void *buffer, unsigned long maxsize) diff --git a/fs/fs.c b/fs/fs.c index e1e4fcc236d6..0b919f286038 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -82,7 +82,7 @@ static struct fstype_info fstypes[] = { .probe = fat_set_blk_dev, .close = fat_close, .ls = file_fat_ls, - .exists = fs_exists_unsupported, + .exists = fat_exists, .read = fat_read_file, .write = fs_write_unsupported, }, diff --git a/include/fat.h b/include/fat.h index 2c951e7d79c6..c8eb7ccd2904 100644 --- a/include/fat.h +++ b/include/fat.h @@ -188,6 +188,7 @@ file_read_func file_fat_read; int file_cd(const char *path); int file_fat_detectfs(void); int file_fat_ls(const char *dir); +int fat_exists(const char *filename); long file_fat_read_at(const char *filename, unsigned long pos, void *buffer, unsigned long maxsize); long file_fat_read(const char *filename, void *buffer, unsigned long maxsize);
On 27 January 2014 13:50, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
This hooks into the generic "file exists" support added in an earlier patch, and provides an implementation for the FAT filesystem.
Signed-off-by: Stephen Warren swarren@nvidia.com
Acked-by: Simon Glass sjg@chromium.org
participants (2)
-
Simon Glass -
Stephen Warren