[U-Boot] [PATCH 1/5] Add bmp_layout module for accessing BMP header data

[Igor Grinberg <grinberg@compulab.co.il>]

On 02/05/13 01:11, Wolfgang Denk wrote:
> Dear Albert ARIBAUD,
> 
> In message <20130204222628.545da91e@lilith> you wrote:
>>
>> The point about md not checking alignment is indeed valid: one should
>> know that a md.l requires a 4-byte-aligned address or it will abort.
> 
> Or, one might do this _intentionally_ for some testing purposes.
> For me it is of utmost importance that U-Boot does not come in my way
> in such things.  It is supposed to do _exactly_ what I ask it to do,
> even if this appears to be a stupid thing.

I agree on this one, except for the case where doing that stupid thing
bricks the board.

> 
>> OTOH, a cautious user may think that to ensure proper alignment, a BMP
>> should be loaded on a 4-byte boundary, but IIUC that it precisely what
>> will cause the load to fail, due to the sole 4-byte field of the BMP
>> header being misaligned by two bytes.
> 
> Sole 4 byte field?  The bitmap file header has actually two 32-bit
> entries: file_size, and data_offset. [The "reserved" entry as we are
> using it should actually be two 16 bit entries, at least according to
> [1]).
> 
> Yes, struct bmp_header is a packed array with non-natural order of
> entries; see also section "Bitmap file header" at [1]; we may consider
> this a really stupid thing to do, but we have to live with this fact.

It was not that stupid in times of DOS and Win 3.1
when int was 16 bits long (and DRAM was 64K or even less)...

> 
> [1] http://en.wikipedia.org/wiki/BMP_file_format
> 
> As I understand the problem comes from the fact, that this issue has
> long been undetected, because the PTB that were/are responsible for
> GCC on ARM had decided that any access to apacked structure would be
> silently broken down into byte accesses, no matter what the actual
> data type was.  While more recent versions of GCC started actually
> attempting 16 or 32 bit accesses, which failed on some systems.
> 
>> So if we leave BMP loading as it is now, the load address will need to
>> be 16-bit-but-not-32-bit-aligned, which is complicated enough to
>> require documentation.
> 
> Indeed, this should be documented.  And eventually the bmp command
> should print a warning message if it sees other alignment.

Agreed on this also, but again what about the board brick case?
Should we add the check for alignment and if it does not properly aligned
deny further bmp header processing along with printing a warning?

> 
>> Or, the BMP struct could be prepended with two bytes so that the
>> load address alignment requirement becomes a simple 4-byte boundary,
>> which most users are... bound... to choose naturally.
> 
> That would violate the "standard" defining the BMP header structure.

Yep, I would not want this to happen.


-- 
Regards,
Igor.