[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Devise macros to encapsulate (x & -x)



On Mon, 20 Nov 2023, Julien Grall wrote:
> On 18/11/2023 02:46, Stefano Stabellini wrote:
> > On Fri, 17 Nov 2023, Andrew Cooper wrote:
> > > On 17/11/2023 10:17 am, Nicola Vetrini wrote:
> > > > Hi all,
> > > > 
> > > > As discussed in this thread [1], which is about complying with MISRA C
> > > > Rule 10.1,
> > > > a macro was introduced to encapsulate a well-known construct:
> > > > 
> > > > /*
> > > >   * Given an unsigned integer argument, expands to a mask where just
> > > > the least
> > > >   * significant nonzero bit of the argument is set, or 0 if no bits are
> > > > set.
> > > >   */
> > > > #define ISOLATE_LSB(x) ((x) & -(x))
> > > > 
> > > > This macro has a gained some calls in the subsequent patches in that
> > > > thread, but concerns were raised around the fact that it would be
> > > > better to devise a macro that evaluates its argument only once. A
> > > > proposed solution is this (thanks to Jan Beulich):
> > > > 
> > > > #define ISOLATE_LSB(x) ({ \
> > > >       typeof(x) x_ = (x); \
> > > >       x_ & -x_; \
> > > > })
> > > 
> > > Of course this was going to explode.
> > > 
> > > This isn't even the first time an unwise attempt to do single-evaluation
> > > has needed to be reverted because it doesn't work with Integer Constant
> > > Expressions.
> > > 
> > > Switch it back to the first form.  It's obviously a macro to begin with,
> > > and not likely to be used in cases that have side effects.
> > 
> > +1
> 
> FWIW +1. I don't much like the idea to have two different versions of the
> helper if there is no real need for it.

Jan, would you be willing to accept that other maintainers have a
preference for having a single MACRO even if suboptimal?

If so, can we go ahead and commit the original patches?

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.