[U-Boot] [PATCH V2] ARM: prevent misaligned array inits

[Andreas Bießmann]

Hi Albert,

On 08.10.2012 16:36, Albert ARIBAUD wrote:
> Hi Andreas,
> 
> On Mon, 08 Oct 2012 13:21:45 +0200, "Andreas Bießmann"
> <andreas.devel at googlemail.com> wrote:
> 
>> Dear Albert Aribaud,
>>
>> On 05.10.2012 21:15, Albert ARIBAUD wrote:
>>> Under option -munaligned-access, gcc can perform local char
>>> or 16-bit array initializations using misaligned native
>>> accesses which will throw a data abort exception. Fix files
>>> where these array initializations were unneeded, and for
>>> files known to contain such initializations, enforce gcc
>>> option -mno-unaligned-access.
>>>
>>> Signed-off-by: Albert ARIBAUD <albert.u.boot at aribaud.net>
>>> ---
>>> V2: fix incorrect doc file name in dabort handler message
>>
>> well, I can not see any change regarding the doc file ;)
> 
> Maybe you though there was a change in the doc file itself? V2 is about
> correcting the file *name* as printed out by the dabort handler.

Well, that was I understood by the V2 comment ...

but V1 has:
---8<---
-	printf ("data abort\n");
+	printf ("data abort\n\n    MAYBE you should read
doc/README.unaligned-access\n\n");
--->8---

and V2 has:
---8<---
-	printf ("data abort\n");
+	printf ("data abort\n\n    MAYBE you should read
doc/README.unaligned-accesses\n\n");
--->8---

However the filename is 'doc/README.arm-unaligned-accesses'

<snip>

>>> +
>>> +Considering the rarity of actual occurrences (as of this writing, 5
>>> +files out of 7840 in U-Boot, or .3%, contain an initialized local char
>>> +array which cannot actually be replaced with a const char*), detection
>>> +if the issue in patches should not be asked from contributors.
>>
>> ---8<---
>> Considering the rarity of actual occurrences, detection if the issue in
>> patches should not be asked from contributors.
>> --->8---
>>
>> I think there is something wrong wih this sentence ... albeit I can not
>> definitely say what it is.
> 
> s/if/of/, but yes, the sentence could use some rewriting. "Considering
> that actual occurrences of the issue are rare, contributors should not
> be required to systematically try and detect the issue in their
> patches". Would that be ok?

Sounds way better.

> 
>>> +
>>> +Detecting files susceptible to the issue can be automated through a
>>> +filter installed as a hook in .git which recognizes local char array
>>> +initializations. Automation should err on the false positive side, for
>>> +instance flagging non-local arrays as if they were local if they cannot
>>> +be told apart.
>>
>> Isn't that a suggestion that should be asked to the list instead? I
>> wonder why it is written down in this README document.
> 
> Not sure I understand what you mean about "ask[ing] the list instead".

I meant to discuss this on the list and just install the hook then (at
least for the denx.de repositories).

> As for writing it down, I did because it's better written down than
> forgotten, and at this point there is no proper way to auto install
> a git hook in a U-Boot developer's tree, at least none that I would
> feel safe pushing so close to a release. So even if I did provide a
> script, each developer would have to manually put the hook in place.
> Therefore, the doc just says how to automate detection.

But you are right, so forget my comment.

best regards

Andreas Bießmann