This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
RE: FW: [PATCH ARC] bfd/arc: Allow non-instruction relocations within .text sections
- From: Cupertino Miranda <Cupertino dot Miranda at synopsys dot com>
- To: Nick Clifton <nickc at redhat dot com>, Cupertino Miranda <Cupertino dot Miranda at synopsys dot com>, "binutils at sourceware dot org" <binutils at sourceware dot org>
- Cc: Claudiu Zissulescu <Claudiu dot Zissulescu at synopsys dot com>, Francois Bedard <Francois dot Bedard at synopsys dot com>
- Date: Tue, 1 Mar 2016 15:34:27 +0000
- Subject: RE: FW: [PATCH ARC] bfd/arc: Allow non-instruction relocations within .text sections
- Authentication-results: sourceware.org; auth=none
- References: <EB86EB452ADE4B44B294F7149B8A27770218E499 at DE02WEMBXB dot internal dot synopsys dot com> <56D5ACF1 dot 50905 at redhat 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