[coreboot] [PATCH] CBFS decompress error

Myles Watson mylesgw at gmail.com
Thu Oct 8 23:06:23 CEST 2009


On Thu, Oct 8, 2009 at 2:56 PM, Patrick Georgi <patrick at georgi-clan.de> wrote:
> Am Donnerstag, den 08.10.2009, 14:49 -0600 schrieb Myles Watson:
>> Move the ulzma prototype into cbfs.h
> Not sure if I like it - that way, ulzma() becomes part of the public
> interface of cbfs.
> Any issue this fixes?
Just the ugliness of having the prototype in the code right before the
call.  I'd be fine with putting it somewhere else.  The reason I put
it where I did is because it is right after the CBFS_COMPRESS_LZMA
define.

>> Check the return value when decompressing.
> That one is nice. Thank you.
>
>> Signed-off-by: Myles Watson <mylesgw at gmail.com>
>>
>> It's not 100% fixed, since the caller (cbfs_load_stage)
>>
>>       if (cbfs_decompress(stage->compression,
>>                            ((unsigned char *) stage) +
>>                            sizeof(struct cbfs_stage),
>>                            (void *) (u32) stage->load,
>>                            stage->len))
>>               return (void *) -1;
>>
>> returns -1 and then cbfs_and_run_core jumps to it.
>>
>> print_debug("Jumping to image.\r\n");
>> dst = cbfs_load_stage(filename);
>> print_debug("Jumping to image.\r\n");
>>
>> Why do we jump to a -1 instead of hanging?
> -1 is a known free value (it points into ROM, at a highly unlikely
> address) and as such used as "invalid image" marker.
It causes my machine to reboot.  I was wondering if that was intentional.

> It should be caught by the caller, which _might_ have some better idea
> than hanging (eg. some backup image), so this is not an option inside
> cbfs_load_stage.
Unless you have normal and fallback?

Thanks,
Myles




More information about the coreboot mailing list