Re: [binutils-gdb] remove pointless assignment

["Maciej W. Rozycki" <macro at imgtec dot com>]

Trevor,

 Why did you commit this change without a review?  I saw Alan made some 
suggestions on top of your original submission, but he didn't say a change 
with these updates was preapproved, and neither your change falls into the 
obviously correct category.  Also in any case you need to post to the 
mailing list the final version committed.

 I have a few notes for you for the future which I would make in the 
review process if you gave me the chance, see below.

> commit 92fce9bd7a4d5732fe9db05b7ebaef4ab858e69a
> Author: Trevor Saunders <tbsaunde+binutils@tbsaunde.org>
> Date:   Wed Apr 13 05:03:22 2016 -0400
> 
>     remove pointless assignment

 Please try to make your patch headings more descriptive, with a clean-up 
like this at least identifying the place where the change is made, so that 
commands like `git shortlog' or `git log --abbrev-commit --pretty=oneline' 
give useful results.  Here e.g.:

MIPS/GAS: Remove pointless assignment in `md_begin'

would do -- with the orignal change that is, as the final one went far 
beyond that.

>     Presumably this was supposed to be regname[sizeof (regname) - 1] but was typoed
>     to regname[sizeof (rename) - 1].  However that should be unnecessary because
>     sprintf should null terminate.  As is this assignment is invalid ISO C because
>     rename refers to the function rename (), and sizeof on functions is undefined.
>     In GNU C C the size of functions is 1 so the expression is the same as
>     regname[0].  The following call to sprintf () clearly will over right that, so
>     the statement either has no effect or is invalid.  Given that it seems safe to
>     just remove it.  While we are there correct the size of regname, and switch
>     from snprintf to sprintf since we know the exact length of the result.

 Please try to keep lines of the commit description at most 74 characters 
wide as with ChangeLog entries, bearing in mind that commands like `git 
show' will indent it by 4 characters.  This is to make them render in a 
readable way on a standard 80-character terminal.

 Your message shows here like this:

    Presumably this was supposed to be regname[sizeof (regname) - 1] but was typ
oed
    to regname[sizeof (rename) - 1].  However that should be unnecessary because
    sprintf should null terminate.  As is this assignment is invalid ISO C becau
se
    rename refers to the function rename (), and sizeof on functions is undefine
d.
[...]

which disturbs reading.

>     gas/ChangeLog:
>     
>     2016-04-15  Trevor Saunders  <tbsaunde+binutils@tbsaunde.org>
>     
>     	* config/tc-mips.c (md_begin): Remove useless assignment.

 Your ChangeLog entry does not correspond to the actual change made.  
Please make sure it remains consistent as you make updates.

 Thank you for your cooperation and your contribution, and happy hacking!

  Maciej