(Resending due to MIME encoding last time. Sorry; I really have to stop
using Outlook for this list for some reason)

On 11/08/2011 09:06 AM, Wolfgang Denk wrote:
> In message <1320164902-24190-1-git-send-email-swar...@nvidia.com> you wrote:
>> image_get_ram_disk() and image_get_kernel() perform operations in a
>> consistent order. Modify image_get_fdt() to do things the same way.
>> This allows a later change to insert some image header manipulations
>> into these three functions in a consistent fashion.
...
>> @@ -1131,14 +1131,19 @@ static const image_header_t *image_get_fdt(ulong 
>> fdt_addr)
>>  {
>>      const image_header_t *fdt_hdr = (const image_header_t *)fdt_addr;
>>  
>> -    image_print_contents(fdt_hdr);
>> +    if (!image_check_magic(fdt_hdr)) {
>> +            fdt_error("fdt header bad magic number\n");
>> +            return NULL;
>> +    }
>>  
>> -    puts("   Verifying Checksum ... ");
>>      if (!image_check_hcrc(fdt_hdr)) {
>>              fdt_error("fdt header checksum invalid");
>>              return NULL;
>>      }
>>  
>> +    image_print_contents(fdt_hdr);
>> +
>> +    puts("   Verifying Checksum ... ");
>>      if (!image_check_dcrc(fdt_hdr)) {
>>              fdt_error("fdt checksum invalid");
>>              return NULL;
> 
> The rule in U-Boot when generating output is to print a message
> before you start an action, and then either print an OK or an error
> message.  The reason for this is debug support: if neither an OK nor
> an error comes you know that the test somehow crashed.
> 
> Here this principle is violated as image_check_magic() and
> image_check_hcrc() will run without being announced.
> 
> Please move the output so we get a message printed before starting to
> perform the actual tests.

The new code is exactly the same as the existing image_get_kernel() and
image_get_ramdisk(). Are those wrong? I wouldn't want to fix my patch to
conform to some supposed standard when the existing code that's been
accepted doesn't conform to that standard, or would I be responsible for
fixing up that too?

But anyway, I imagine there's no point discussing this patch further,
because its sole purpose is to support the uImage load_address=-1 patch
that follows, and it's pretty clear that won't be accepted, so please
consider this patch series withdrawn too.

-- 
nvpublic
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to