Re: [PATCH V2 2/7] binutils: Repair nios2 PLT and GP handling

[Marek Vasut <marex@denx.de>]

On Tuesday, February 09, 2016 at 05:00:11 PM, Randy MacLeod wrote:
> On 2016-02-09 09:29 AM, Marek Vasut wrote:
> > On Tuesday, February 09, 2016 at 03:21:17 PM, Burton, Ross wrote:
> >> On 9 February 2016 at 13:08, Marek Vasut <marex@denx.de> wrote:
> >>> Repair corner cases in nios2 PLT and GP handling. See the patches
> >>> themselves for extensive detailed explanation.
> 
> This sentence doesn't really add much value since many patches
> contain additional description and the logs here are mostly a
> few lines from the ChangeLog which doesn't count as an extensive
> detailed explanation in my books. :)

It's not from changelog, it's from the commit message. The real answer
is that it fixes incorrect computation of the GOT/PLT offsets in case
these tables grow too large. Then there's some sort of off-by-four error
in the computation, which causes linking to fail. It happens in one or
two packages in yocto. I had these patches in my tree for a bit, so my
memory is a little unclear on the gruesome details already.

> I expect that you're familiar with this but take a look at:
> http://www.openembedded.org/wiki/How_to_submit_a_patch_to_OpenEmbedded#Comm
> itting_your_patch
> 
>      Commit with a concise and descriptive message -
>      one that explains your changes in a way others get
>      a short overview without looking at the code.

Yes, I hope it's there.

> >>> The patches don't contain the ChangeLog entries, since the changelog
> >>> entries often cause trouble when applying the patch.
> 
> I'd prefer to see the ChangeLog snippets duplicated in the git log
> so that it's easier to see what happened.

You actually want to avoid that, since it'd make it really hard to apply
these patches onto subsequent versions of binutils if this was necessary.
I hope this won't be necessary and will be dropped once the next version
of binutils arrives. The change which happened is in the patch's commit
message, but it seems like some of the binutils guys are not very fond of 
writing extensive explanation.

> If you do a v2, please also mention that the patches are backports
> since that's important to people when they consider whether the patch
> should be accepted.

This is a V2 ;-)

There is "Upstream-Status: Backport" for each of the patches, it's actually
needed to track the status.

Best regards,
Marek Vasut