[U-Boot] [PATCH] display_buffer: fix misaligned buffer

[Reinhard Meyer]

Hi Detlev,
>> diff --git a/lib/display_options.c b/lib/display_options.c
>> index 20319e6..9048a8a 100644
>> --- a/lib/display_options.c
>> +++ b/lib/display_options.c
>> @@ -101,7 +101,7 @@ void print_size(unsigned long long size, const char *s)
>>  #define DEFAULT_LINE_LENGTH_BYTES (16)
>>  int print_buffer (ulong addr, void* data, uint width, uint count, uint linelen)
>>  {
>> -	uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1];
>> +	uint32_t linebuf[MAX_LINE_LENGTH_BYTES/4 + 1];
>>  	uint32_t *uip = (void*)linebuf;
>>  	uint16_t *usp = (void*)linebuf;
>>  	uint8_t *ucp = (void*)linebuf;
> 
> Sorry to jump in here late, but I do not like this change.  How can a
> reader of the code who has not followed the discussion here infer that
> the datatype is there to ensure alignment?
> 
> I am willing to bet at least a few beers that it will not take long
> until someone posts a patch changing the datatype back, because
> c-strings are bytes.
> 
> I would much rather see an alignment attribute, which will _document_
> the problem _and_ fix it, instead of only fixing it.

One could add a comment above like:
	/*
	 * it is mandatory that linebuf stays uint32_t aligned
	 * since we are going to slide along it with a uint32_t
	 * pointer
	 */
	uint32_t linebuf[MAX_LINE_LENGTH_BYTES/4 + 1];

I personally prefer this above an attribute. Its disputeable but I prefer
to do things with "normal C constructs" where possible. You can already
see from the discussion that __aligned as a toolchain-abstracted
variant (defined in a toolchain header file) or attribute((__aligned__))
as a very toolchain dependant variant shall be used ;)

Anyway, both patches have been offered, any will work for me as long as
I can see ASCII properly on ARM machines...

without patch:
22000000: 41424344 41424344 41424344 41424344    ADCBADCBADCBAV4.
with patch:
22000000: 41424344 41424344 41424344 41424344    DCBADCBADCBADCBA

Reinhard