[U-Boot] [PATCH 2/2] Add unlzo command

[Joe Hershberger]

Hi Mike,

On Fri, Aug 17, 2012 at 6:26 PM, Mike Frysinger <vapier at gentoo.org> wrote:
> On Friday 17 August 2012 16:59:44 Joe Hershberger wrote:
>> --- a/common/Makefile
>> +++ b/common/Makefile
>>
>>  ifdef CONFIG_LZMA
>>  COBJS-$(CONFIG_CMD_UNLZMA) += cmd_unlzma.o
>>  endif
>> +ifdef CONFIG_LZO
>> +COBJS-$(CONFIG_CMD_UNLZO) += cmd_unlzo.o
>> +endif
>
> imo, these ifdefs shouldn't exist.  the commands shouldn't get silently
> ignored because someone omitted an option.  add an #ifdef check to the .c file
> and have it #error out if the necessary config options aren't defined.

OK

>> --- /dev/null
>> +++ b/common/cmd_unlzo.c
>>
>> +int do_unlzo(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>
> static

OK

>> +     ret = lzop_decompress((void *)src, src_len, (void *)dst, &dst_len);
>> +     if (ret != LZO_E_OK) {
>> +             printf("unlzo: uncompress or overwrite error %d\n", ret);
>> +             return -1;
>
> how about returning ret ?

OK

>> +     sprintf(buf, "%lX", (unsigned long) dst_len);
>> +     setenv("filesize", buf);
>
> setenv_ulong() ?

OK

>> +U_BOOT_CMD(
>> +     unlzo,  5,      1,      do_unlzo,
>> +     "unlzo a memory region",
>> +     "srcaddr srcsize dstaddr [dstsize]"
>> +);
>
> isn't there a way you could "stream" this so you don't need the srcsize ?  or
> does the lzop API not support that ?

The lzo API uses the source size to check if the input is complete at
the end of the file.  I could pass in ~0 optionally and it would stop
at the end.  (It's not as clear how unlzma will handle it.)

So it would look like "unlzo srcaddr dstaddr [dstsize]"

Thanks,
-Joe