FW: [PATCH ARC] bfd/arc: Allow non-instruction relocations within .text sections
Claudiu Zissulescu
Claudiu.Zissulescu@synopsys.com
Tue Mar 8 13:28:00 GMT 2016
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
More information about the Binutils
mailing list