Dear Timur,
in message 11782314361072-git-send-email-timur@freescale.com you wrote:
The do_bootm_linux() function uses the same variable ('len') to calculate the dtu (flat device tree wrapped in a uImage) length and the initrd length, which means that the initrd length was incorrect when it was needed. This patch changes any code that uses 'len' or 'data' as temporary variables to use the values directly instead. It also adds a comment to the definitions of 'len' and 'data' reminding other programmers what they represent.
...
This patch is a simplied version of the initrd patch that only fixes the root problem, without renaming any variables. This bug was introduced in commit 87a449c8ac396420cb24260f717ea9e6faa82047.
...
@@ -523,11 +523,11 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag, int verify) { ulong sp;
- ulong len, checksum;
- ulong len, data; /* The initrd length and address */ ulong initrd_start, initrd_end; ulong cmd_start, cmd_end; ulong initrd_high;
- ulong data;
- ulong checksum; int initrd_copy_to_ram = 1; char *cmdline; char *s;
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.
@@ -652,13 +652,10 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag, do_reset (cmdtp, flag, argc, argv); }
data = (ulong)&header;len = sizeof(image_header_t);checksum = ntohl(hdr->ih_hcrc); hdr->ih_hcrc = 0;
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.
@@ -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.
With this patch it is impossible to see what the problem is you are trying to fix.
Looking at the patch, I see only cosmetical changes, which actually lead to less readable code.
If there is a problem, please fix the problem, but don't touch innocent code that has been there for ages.
Best regards,
Wolfgang Denk