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

[Cupertino Miranda <Cupertino dot Miranda at synopsys dot com>]

Hi Nick,

Thanks for the review.

To be honest don't know how this formatting mistakes went through in this patch. Will fix them and request Claudiu to push it. :-)
The braces in the example you mentioned are a security measure.
Finding that the macro is expanded to multiple non braced statements after hours of debugging is no fun. :-) 

Best regards,
Cupertino

> -----Original Message-----
> From: Nick Clifton [mailto:nickc@redhat.com]
> Sent: Tuesday, March 01, 2016 3:54 PM
> To: Cupertino Miranda; binutils@sourceware.org
> Cc: Claudiu Zissulescu; Francois Bedard
> Subject: 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