Hi Simon,
On 12/17/2014 12:02 AM, Simon Glass wrote:
Hi Nikolay,
On 12 December 2014 at 11:01, picmaster@mail.bg wrote:
From: Nikolay Dimitrov picmaster@mail.bg
Fix issue in parse_verify_sum() which swaps handling of env-var and *address. Move hash_command() argc check earlier. Cosmetic change on do_hash() variable declaration. Improved help message for "hash" command.
Signed-off-by: Nikolay Dimitrov picmaster@mail.bg
Thanks for this. Main change looks good, a few nits.
common/cmd_hash.c | 28 +++++++++++++--------------- common/hash.c | 6 ++---- 2 files changed, 15 insertions(+), 19 deletions(-)
diff --git a/common/cmd_hash.c b/common/cmd_hash.c index 90facbb..704d21e 100644 --- a/common/cmd_hash.c +++ b/common/cmd_hash.c @@ -18,9 +18,9 @@ static int do_hash(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { char *s; -#ifdef CONFIG_HASH_VERIFY int flags = HASH_FLAG_ENV;
+#ifdef CONFIG_HASH_VERIFY if (argc < 4) return CMD_RET_USAGE; if (!strcmp(argv[1], "-v")) { @@ -28,8 +28,6 @@ static int do_hash(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) argc--; argv++; } -#else
const int flags = HASH_FLAG_ENV;@@ -40,19 +38,19 @@ static int do_hash(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) }
#ifdef CONFIG_HASH_VERIFY -U_BOOT_CMD(
hash, 6, 1, do_hash,"compute hash message digest","algorithm address count [[*]sum_dest]\n"" - compute message digest [save to env var / *address]\n""hash -v algorithm address count [*]sum\n"" - verify hash of memory area with env var / *address"-); +#define HARGS 6 #else +#define HARGS 5 +#endif
- U_BOOT_CMD(
hash, 5, 1, do_hash,"compute message digest","algorithm address count [[*]sum_dest]\n"
hash, HARGS, 1, do_hash,"compute hash message digest","algorithm address count [[*]hash_dest]\n" " - compute message digest [save to env var / *address]"-); +#ifdef CONFIG_HASH_VERIFY
"\nhash -v algorithm address count [*]hash\n"" - verify message digest of memory area to immediate value, \n"Perhaps " verify message digest of memory area, display result or write to env var or *address"
I think that the initial description was appropriate, because the "hash" command supports 3 modes of operation (I chose crc32 for shorter examples):
1. Verify hash against immediate value, like this
hash -v crc32 0x20000000 $filesize e9b11acd
2. Verify hash against environment variable, which holds the actual hash value
hash -v crc32 0x20000000 $filesize env_var_name
3, Verify hash against value, placed at specific memory address
hash -v crc32 0x20000000 $filesize *0x30000000
As far as I observed the code and its behavior, the only case when the "hash -v" result was printed was when the verification failed. Please correct me if I'm wrong.
#endif +); diff --git a/common/hash.c b/common/hash.c index 12d6759..aceabc5 100644 --- a/common/hash.c +++ b/common/hash.c @@ -256,7 +256,7 @@ static int parse_verify_sum(struct hash_algo *algo, char *verify_str, env_var = 1; }
if (env_var) {
if (!env_var) { ulong addr; void *buf;@@ -347,7 +347,7 @@ int hash_command(const char *algo_name, int flags, cmd_tbl_t *cmdtp, int flag, { ulong addr, len;
if (argc < 2)
if ((argc < 2) || ((flags & HASH_FLAG_VERIFY) && (argc < 3))) return CMD_RET_USAGE; addr = simple_strtoul(*argv++, NULL, 16);@@ -380,8 +380,6 @@ int hash_command(const char *algo_name, int flags, cmd_tbl_t *cmdtp, int flag, #else if (0) { #endif
if (!argc)return CMD_RET_USAGE;What does this change achieve?
I moved the verification earlier, because the original implementation first calculated the hash and just then complained about the last missing argument. My personal understanding is that errors should be caught as early as possible, and work should be done only as necessary :D.
if (parse_verify_sum(algo, *argv, vsum, flags & HASH_FLAG_ENV)) { printf("ERROR: %s does not contain a valid "-- 1.7.10.4
Regards, Simon
Kind regards, Nikolay