[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