Re: [PATCH 7/7] command: add generic digest command

[Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>]

> > +
> > +static int do_digest(int argc, char *argv[])
> > +{
> > +	struct digest *d;
> > +	unsigned char *tmp_key = NULL;
> > +	unsigned char *tmp_sig = NULL;
> > +	char *sig = NULL;
> > +	char *sigfile = NULL;
> > +	size_t siglen = 0;
> > +	char *key = NULL;
> > +	char *keyfile = NULL;
> > +	size_t keylen = 0;
> > +	size_t digestlen = 0;
> > +	char *algo = NULL;
> > +	int opt, ret;
> > +
> > +	if (argc < 2) {
> > +		__prints_algo();
> > +		return 0;
> > +	}
> 
> This is an untuitive trigger to print the available algos. Can we add an
> explicit option here?

I would have prefer via help cmd but not possible to have runtime help txt
> 
> > +
> > +	while((opt = getopt(argc, argv, "a:k:K:s:S:")) > 0) {
> > +		switch(opt) {
> > +		case 'k':
> > +			key = optarg;
> > +			keylen = strlen(key);
> > +			break;
> > +		case 'K':
> > +			keyfile = optarg;
> > +			break;
> > +		case 'a':
> > +			algo = optarg;
> > +			break;
> > +		case 's':
> > +			sig = optarg;
> > +			siglen = strlen(sig);
> > +			break;
> > +		case 'S':
> > +			sigfile = optarg;
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (!algo)
> > +		return COMMAND_ERROR_USAGE;
> > +
> > +	d = digest_alloc(algo);
> > +	if (!d) {
> > +		eprintf("algo '%s' not found\n", algo);
> > +		__prints_algo();
> > +		return COMMAND_ERROR_USAGE;
> > +	}
> > +
> > +	argc -= optind;
> > +	argv += optind;
> > +
> > +	if (keyfile) {
> > +		tmp_key = key = read_file(keyfile, &keylen);
> 
> Why two variables? Both tmp_key and key are never changed.
'key' can be from optarg so we can not free it

otherwise if need to xstrdup it when parsing the getopt
> 
> > +		if (!key) {
> > +			eprintf("file '%s' not found\n", keyfile);
> > +			goto err;
> > +		}
> > +	}
> > +
> > +	digest_set_key(d, key, keylen);
> 
> This can fail. You should check the error code.
yeah
> 
> > +unsigned char to_digit(unsigned char c)
> > +{
> > +	if (c >= '0' && c <= '9')
> > +		c -= '0';
> > +	else
> > +		c -= 'a' - 10;
> > +
> > +	return c;
> > +}
> > +
> > +unsigned char to_hexa(unsigned char c)
> > +{
> > +	if (c < 10)
> > +		c += '0';
> > +	else
> > +		c += 'a' - 10;
> > +
> > +	return c;
> > +}
> > +
> > +int base64_to_hex(const unsigned char *sum, unsigned char *buf, size_t length)
> > +{
> 
> The ASCII input here contains hex digits, base64 is something different.
> Also these functions are useful enough to be always available, not only
> when digest is enabled.
> 
> I just sent a patch containing the kernels implementation of bin2hex and
> hex2bin. Please base on this one.

ok

Best Regards,
J.

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