Re: [PATCHv2] commands: add md5/sha1/sha256sum commands using the digest api

[Peter Korsgaard <jacmet@sunsite.dk>]

>>>>> "Jean-Christophe" == Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> writes:

Hi,

 >> +
 >> +	data = xmalloc(d->length);

 Jean-Christophe> we realloc a buffer here where we alloc one in file_digest
 Jean-Christophe> and not free it before calling print_digest

I'm not exactly sure what you mean, but yes - We could inline
print_digest in file_digest and reuse the 4K buf for the digest as
well. That would shrink the code slightly (lines / bytes) and get rid of
the 2nd xmalloc.

I'll fix that and send a v3 shortly.

 >> +	buf = xmalloc(4096);

 Jean-Christophe> do we really need to allocate the buffer everytime?

Not really, but it's simpler to do like this. It's only once per file so
hardly a performance issue.

 >> +	while (*argv) {
 >> +		char *filename = "/dev/mem";
 >> +		ulong start = 0, size = ~0;

 Jean-Christophe> ulong so so if we want to support 64bit vfs we need to
 Jean-Christophe> use an other type

ulong is what parse_area_spec and the other memory commands use.

 >> +#ifdef CONFIG_CMD_MD5SUM
 >> +
 >> +static int do_md5(struct command *cmdtp, int argc, char *argv[])
 >> +{
 >> +	return do_digest("md5", argc, argv);

 Jean-Christophe> pass the cmdtp will be good
 Jean-Christophe> I agree it's not necessary now but I think it's a good
 Jean-Christophe> practice

I think it makes more sense to add it when (if?) it is needed.

-- 
Bye, Peter Korsgaard

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