This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: FW: [PATCH ARC] bfd/arc: Allow non-instruction relocations within .text sections


Hi Cupertino,

> Can you please also provide us feedback for this patch as well. ;-)

Certainly...

  There are some formatting issues:

  * Multi-line expressions connected by boolean or binary operations 
    should have those operations appear at the start of a line, rather
    than the end of a line.  Thus:

#define IS_ME(FORMULA,BFD) ((strstr(FORMULA, "ME") != NULL) && \
                            (!bfd_big_endian (BFD)))

    should be:

#define IS_ME(FORMULA,BFD) ((strstr(FORMULA, "ME") != NULL) \
                            && (!bfd_big_endian (BFD)))


  * A similar rule applies to braces.  So:

  if (strstr (#FORMULA, " ME ") != NULL) { \
      BFD_ASSERT (SIZE == 2); \
    }

    Should be:

  if (strstr (#FORMULA, " ME ") != NULL) \
    { \
      BFD_ASSERT (SIZE == 2); \
    }

    Or even just:

  if (strstr (#FORMULA, " ME ") != NULL) \
    BFD_ASSERT (SIZE == 2);


   If you prefer.


  * Also when a function (or macro) is invoked its name should be 
    separated from the opening parenthesis by a space.  Hence:

#define IS_ME(FORMULA,BFD) ((strstr(FORMULA, "ME") != NULL) \
                            && (!bfd_big_endian (BFD)))

    Should be:

#define IS_ME(FORMULA,BFD) ((strstr (FORMULA, "ME") != NULL) \
                            && (!bfd_big_endian (BFD)))



But apart from that the patch looks OK to me.

Cheers
  Nick


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]