Nick Clifton nickc@redhat.com
Mon Mar 23 17:30:11 GMT 2020

• Previous message (by thread): Commit: wrstabs.c: replace assertions with error returns
• Next message (by thread): readelf memory leak in process_mips_specific
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

---

Hi Dmitrij,

  [Sorry for the long delay in replying to your email].

  Thanks for the patch.  It is looking good, but there are several
  things missing.  Speficially:

    * A ChangeLog entry to describe the patch.
    * An update to the gas/NEWS file mentioning the new directives.
    * A patch to gas/doc/as.texi describing the new directives.
    * A new test or two in the gas testsuite to check the behaviour of
      the new macros and to make sure that they continue to work in the
      future.
    * Formatting the code to follow the GNU Coding Standards
       https://www.gnu.org/prep/standards/

Cheers
  Nick

PS.  With regard to the GNU Coding Standards I am particularly
  thinking of the following:

    static int is_float(const char* str) {

  which should be:

    static int
    is_float (const char* str)
    {

  (Ie put the return type on a separate line, as well as the
  opening curly brace).

  Plus:

     switch (arg) {

  Which should be:
 
     switch (arg)
     {

  (Opening curly braces are almost always on a line on their own).

  Also - not really a coding standard thing, but helpful nonetheless:

    s_ifnumber (int arg)
    {
       (void)arg;

  Is better written as:

   s_ifnumber (int arg ATTRIBUTE_UNUSED)
   {

  I hope that this helps.

---

• Previous message (by thread): Commit: wrstabs.c: replace assertions with error returns
• Next message (by thread): readelf memory leak in process_mips_specific
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

---

More information about the Binutils mailing list