Re: [Xen-devel] [PATCH 1/7] Port WARN_ON_ONCE() from Linux

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Feb 09, 2018 at 10:45:25AM +0000, Julien Grall wrote:
> Hi,
> 
> On 02/09/2018 10:29 AM, Roger Pau Monné wrote:
> > On Thu, Feb 08, 2018 at 08:10:49PM -0700, Sameer Goel wrote:
> > > diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
> > > index 1d9771340c..697212a061 100644
> > > --- a/xen/include/xen/lib.h
> > > +++ b/xen/include/xen/lib.h
> > > @@ -11,6 +11,19 @@
> > >   #define BUG_ON(p)  do { if (unlikely(p)) BUG();  } while (0)
> > >   #define WARN_ON(p) do { if (unlikely(p)) WARN(); } while (0)
> > > +#define WARN_ON_ONCE(p)                             \
> > > +({                                                  \
> > > +    static bool __section(".data.unlikely") __warned; \
> > > +    int __ret_warn_once = !!(p);                    \
> >         ^ bool
> > 
> > > +                                                    \
> > > +    if ( unlikely(__ret_warn_once && !__warned) )     \
> > > +    {                                               \
> > > +        __warned = true;                            \
> > > +        WARN();                                     \
> > > +    }                                               \
> > > +    unlikely(__ret_warn_once);                      \
> > 
> > Does this macro really need to return something? It seems weird to me
> > to allow usages like: if ( WARN_ON_ONCE...
> 
> This construct is used in Linux (included in the driver ported):
> 
> if (WARN_ON_ONCE(fwspec->iommu_priv)) {
>      master = fwspec->iommu_priv;
>      smmu = master->smmu;
> } else {
> ....
> }
> 
> IHMO the makes the code nicer to read over:

OK, if that's intended I'm fine with it, just wanted to check.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel