Hi Andreas,
On Mon, 08 Oct 2012 16:56:41 +0200, "Andreas Bießmann"
<andreas.de...@googlemail.com> wrote:
> 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.de...@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.b...@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'
Argh!
Will fix in V3... For good. Thanks.
> <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.
Will put in V3.
> >
> >>> +
> >>> +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.
Thanks.
> best regards
>
> Andreas Bießmann
Amicalement,
--
Albert.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
