[U-Boot-Users] [PATCH v3] Fix initrd length miscalculation in bootm command when using a dtu

[Timur Tabi]

Wolfgang Denk wrote:

> So before your patch we had "len", "checksum", and "data", and now we
> have "len", "data", and "checksum".
> 
> What is your reason for this change? I see  no  functinal  difference
> nor any improvement of readability.

It's for the comment that I added.  I wanted to make sure that the reader understood what 
'len' and 'data' are for, since their purpose is not obvious from just reading the code.

>> -		if (crc32 (0, (uchar *)data, len) != checksum) {
>> +		if (crc32 (0, (uchar *) &header, sizeof(image_header_t)) != checksum) {
> 
> This looks functionally identical to me, but is less readable.

The point behind the patch is to make sure that 'len' and 'data' are only used for the 
purpose they are intended.  Here, len and data are used as temporary variables.


> 
>> @@ -779,9 +776,8 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
>>
>>  			checksum = ntohl(hdr->ih_dcrc);
>>  			addr = (ulong)((uchar *)(hdr) + sizeof(image_header_t));
>> -			len = ntohl(hdr->ih_size);
>>
>> -			if(checksum != crc32(0, (uchar *)addr, len)) {
>> +			if(checksum != crc32(0, (uchar *)addr, ntohl(hdr->ih_size))) {
> 
> Same here. functionally identical but less readable.

Not true.  In this case, len has already been assigned the length of the initrd, and here 
we lose this value.  Without this change, the kernel will panic at boot.

> With this patch it is impossible to see what the problem is  you  are
> trying to fix.

It's clearly explained in the comment: 'data' and 'len' are the address and length of the 
initrd, so this patch makes sure that these variables only contain those values.


-- 
Timur Tabi
Linux Kernel Developer @ Freescale