[PATCH v2] drm/xe: Fix build error for XE_IOCTL_DBG macro

[gyeyoung]

> >--- a/drivers/gpu/drm/xe/xe_macros.h
> >+++ b/drivers/gpu/drm/xe/xe_macros.h
> >@@ -11,8 +11,12 @@
> > #define XE_WARN_ON WARN_ON
> >
> > #define XE_IOCTL_DBG(xe, cond) \
> >-      ((cond) && (drm_dbg(&(xe)->drm, \
> >-                          "Ioctl argument check failed at %s:%d: %s", \
> >-                          __FILE__, __LINE__, #cond), 1))
> >+({ \
> >+      if ((cond)) \
> >+              drm_dbg(&(xe)->drm, \
> >+                      "Ioctl argument check failed at %s:%d: %s", \
> >+                      __FILE__, __LINE__, #cond); \
> >+      (cond); \
>
> there's a double cond evaluation here and given any expression can be
> given to XE_IOCTL_DBG(), this doens't look very safe. I think this would
> be safer as:
>
> #define XE_IOCTL_DBG(xe, cond) ({                                       \
>          int cond__ = !!(cond);                                          \
>          if (cond__)                                                     \
>                  drm_dbg(&(xe)->drm,                                     \
>                          "Ioctl argument check failed at %s:%d: %s",     \
>                          __FILE__, __LINE__, #cond);                     \
>          cond__;                                                         \
> })
>
> as it then evaluates cond just once. Also the generated code seems to be
> sane compared to what we had before too.
>

Yes, if (cond) has operator like ++, it will be a bug. I miss that...
I will revise a patch again by referring to your review, thanks.

> And I also needed this to build-test:
>
> | diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> | index 08cfea04e22bd..82585d442f017 100644
> | --- a/drivers/gpu/drm/drm_print.c
> | +++ b/drivers/gpu/drm/drm_print.c
> | @@ -215,9 +215,8 @@ void __drm_printfn_dbg(struct drm_printer *p, struct va_format *vaf)
> |  {
> |         const struct drm_device *drm = p->arg;
> |         const struct device *dev = drm ? drm->dev : NULL;
> | -       enum drm_debug_category category = p->category;
> |
> | -       if (!__drm_debug_enabled(category))
> | +       if (!__drm_debug_enabled(p->category))
> |                 return;
> |
> |         __drm_dev_vprintk(dev, KERN_DEBUG, p->origin, p->prefix, vaf);
>
> as otherwise it complains category is unused.
>

I also submitted a seperate patch to fix '__drm_debug_enabled' macro,
from '#define __drm_debug_enabled(category)  true'
to     '#define __drm_debug_enabled(category)  ({ void(category);  true; })'
This removes the build error caused by the unused 'category', too.

Anyway, it can be build. I tested both cases.
I realize now that these two patches should have been submitted as a
patch series
I'm sorry for my mistakes.

Thanks,
Gyeyoung Baek

> Lucas De Marchi