[PATCH] _bfd_mips_elf_final_link: notify user about wrong .reginfo size
Maciej W. Rozycki
macro@mips.com
Tue Jan 9 01:30:00 GMT 2018
Hi Vlad,
Thank you for your contribution. I will be happy to accept your change
for binutils, however your submission has a few problems, which will have
to be corrected first.
First of all, please explain why this change is needed -- is there a way
to trigger this assertion with user input? This explanation will become
the patch description once the change has been committed, and is subject
to prior review just as the code update itself. See `git log' for
examples of how such a description might look like.
When writing the description please follow text formatting rules as per
the GNU Coding Standard, in particular please keep your text within 74
columns (I recommend 72 columns due to the indentation applied by `git
show', etc.) and place two spaces between a full stop and any following
sentence. For a complete explanation of the rules see:
<https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Formatting_Your_Code>
and:
<https://www.gnu.org/prep/standards/standards.html#Formatting>.
Please note that sentences need to start with a capital letter; this also
applies to the patch heading in the subject.
Second, if this assertion is indeed user-triggerable, then please
describe how to trigger it. We'll need this to create a test suite case
(if feasible), so that we can verify in the future that the error
situation continues being correctly handled.
This will also help double-checking that the error is indeed issued from
the right place -- here it is the output section that is being handled, so
offhand it looks to me like the size should have been set to the correct
value as the output section was created by BFD. If the incorrect size has
been instead copied from an input section, then I think the size mismatch
should be reported in input section processing rather than here.
Third, you need to include a ChangeLog entry at the end of your patch
description. See bfd/ChangeLog for existing examples and:
<https://www.gnu.org/prep/standards/standards.html#Style-of-Change-Logs>
for a full explanation.
Fourth, there are a couple of formatting issues with the change itself;
see below for details.
> diff --git a/bfd/elfxx-mips.c b/bfd/elfxx-mips.c
> index 6a4d3e1a6a..e27327c929 100644
> --- a/bfd/elfxx-mips.c
> +++ b/bfd/elfxx-mips.c
> @@ -14431,7 +14431,13 @@ _bfd_mips_elf_final_link (bfd *abfd, struct bfd_link_info *info)
> }
>
> /* Size has been set in _bfd_mips_elf_always_size_sections. */
> - BFD_ASSERT(o->size == sizeof (Elf32_External_RegInfo));
> + if (o->size != sizeof (Elf32_External_RegInfo)) {
A conditional block's opening brace has to be on the following line,
indented by 2 spaces. The body of the block has to be indented by further
2 spaces (use a tab for every block of 8 spaces).
> + _bfd_error_handler
> + (_("%B: .reginfo section size should be %d bytes, actual size is %d"),
Please wrap the string to stay within 74 columns, as per formatting rules
referred above. See existing examples of `_bfd_error_handler' invocations
throughout this file.
> + abfd, sizeof (Elf32_External_RegInfo), o->size);
> +
> + return FALSE;
> + }
A block's closing brace has to be placed on the same column as the
matching opening brace.
Please resubmit with these issues corrected, or feel free to ask me if
you have any questions.
Maciej
More information about the Binutils
mailing list