Re: [PATCH] commands/digest: add verify support

[Sascha Hauer <s.hauer@pengutronix.de>]

Hi Hubert,

On Mon, Feb 16, 2015 at 02:02:35PM +0100, h.feurstein@gmail.com wrote:
> From: Hubert Feurstein <h.feurstein@gmail.com>
> 
> Signed-off-by: Hubert Feurstein <h.feurstein@gmail.com>
> [EB]: reworked based on Sascha's comments and tested with md5sum
> Signed-off-by: Eric Bénard <eric@eukrea.com>
> ---
> -	if (argc < 2)
> -		return COMMAND_ERROR_USAGE;
> +	while ((opt = getopt(argc, argv, "v:V:")) > 0) {
> +		switch (opt) {
> +		case 'v':
> +			verify++;
> +			free(verify_hash);
> +			hash = xstrdup(optarg);
> +			len = strlen(hash);
> +			break;
> +		case 'V':
> +			verify++;
> +			free(verify_hash);
> +			hash = read_file(optarg, &bufsz);
> +			len = bufsz;
> +			break;
> +		default:
> +			ret = COMMAND_ERROR_USAGE;
> +			goto out;
> +		}
> +
> +		if (verify == 1 && (d->length * 2) <= len && hash != NULL) {
> +			min_argc += 2;
> +			verify_hash = parse_hash(d->length, hash);
> +			if (!verify_hash)
> +				return -ENOMEM;
> +			free(hash);
> +		} else
> +			return COMMAND_ERROR_USAGE;

This code in the option parsing loop looks odd. Could you just parse
options in the loop, store the hash(file) in a variable and do the
processing outside the loop?

> +	}
> +
> +	if (argc < min_argc) {
> +		ret = COMMAND_ERROR_USAGE;
> +		goto out;
> +	}
> +
> +	argv += min_argc - 2;
>  
>  	hash = calloc(d->length, sizeof(unsigned char));

xzalloc should be safe here.

>  	if (!hash) {
>  		perror("calloc");
> -		return COMMAND_ERROR_USAGE;
> +		ret = -ENOMEM;
> +		goto out;
>  	}
>  
>  	argv++;
> @@ -60,17 +133,32 @@ static int do_digest(char *algorithm, int argc, char *argv[])
>  		if (digest_file_window(d, filename, hash, start, size) < 0) {
>  			ret = 1;
>  		} else {
> -			for (i = 0; i < d->length; i++)
> +			for (i = 0; i < d->length; i++) {
>  				printf("%02x", hash[i]);
> +				if (verify > 0 && hash[i] != verify_hash[i])
> +					verify = -1;
> +			}
>  
> -			printf("  %s\t0x%08llx ... 0x%08llx\n",
> +			printf("  %s\t0x%08llx ... 0x%08llx",
>  				filename, start, start + size);
> +
> +			if (verify < 0) {
> +				printf(" ** ERROR ** ");

Printing the real hash and the expected hash would be nice here.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox