On 22.02.2012 10:29, Stefano Babic wrote:
On 22/02/2012 09:14, Dirk Behme wrote:
On 21.02.2012 22:49, stefano babic wrote:
Am 21/02/2012 20:18, schrieb Dirk Behme:
Hi Dirk,
What do you think about anything like below then [1]?
I looked through the imximage.c code and, well, due to the mixture to support the v1 and v2 header format, the execution path isn't the cleanest one. So, while it doesn't seem to be the cleanest way to exit directly in set_imx_hdr_v2, it seems to be the easiest and best place to add this check. Some other functions have some exit() calls, too, so it seems to be common practice in this code.
It is common in all mkimage - when there is an error, it makes no sense to go on.
You must also fix this issue for V1 in set_imx_hdr_v1() as well, because we do not want default value at all.
Ok, the V1 topic is new.
I can't touch V1 because I don't know anything about it. And I don't have any hardware to test anything V1 related.
Even though the V1 code might have a similar issue, it's my understanding that it doesn't hurt there as in V1 there are no flash_offsets != FLASH_OFFSET_STANDARD. Therefore in V1 the existing code works fine (?). Same as the V2 code before Freescale introduced flash offsets which are not FLASH_OFFSET_STANDARD (== 0x400).
I suggest also you do not check with if(imxhdr->flash_offset == 0), in case Freescale will put a SOC without an offset in the future. But it is easy to add a value that is not allowed. If we add something like
FLASH_OFFSET_UNDEFINED = 0xFF
or whatever you want that is not 32-bit aligned, we are on the safest side.
I will look where the correct location might be to add this.
If this is ok, I will send a v2 of the patch.
I will try to update the V2 header with something like FLASH_OFFSET_UNDEFINED as proposed above and then send a v2 of the patch.
Best regards
Dirk