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


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


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