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: Claudiu Zissulescu <Claudiu dot Zissulescu at synopsys dot com>
- To: Cupertino Miranda <Cupertino dot Miranda at synopsys dot com>, Nick Clifton <nickc at redhat dot com>, "binutils at sourceware dot org" <binutils at sourceware dot org>
- Cc: Francois Bedard <Francois dot Bedard at synopsys dot com>, "andrew dot burgess at embecosm dot com" <andrew dot burgess at embecosm dot com>
- Date: Tue, 8 Mar 2016 13:28:32 +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> <EB86EB452ADE4B44B294F7149B8A27770218E7CC at DE02WEMBXB dot internal dot synopsys dot com>
Patch committed.
Regards,
Claudiu
> -----Original Message-----
> From: Cupertino Miranda [mailto:cmiranda@synopsys.com]
> Sent: Tuesday, March 01, 2016 4:34 PM
> To: Nick Clifton; 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 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