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

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

Posted by Gyeyoung Baek 1 year, 3 months ago

In the previous code, there is build error.
if CONFIG_DRM_USE_DYNAMIC_DEBUG is set,
'drm_dbg' function is replaced with '__dynamic_func_call_cls',
which is replaced with a do while statement.

The problem is that,
XE_IOCTL_DBG uses this function for conditional expression.

so I fix the expression to be compatible with the do while statement,
by referring to "https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html".

Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
---
 drivers/gpu/drm/xe/xe_macros.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_macros.h b/drivers/gpu/drm/xe/xe_macros.h
index daf56c846d03..58a9d1e33502 100644
--- a/drivers/gpu/drm/xe/xe_macros.h
+++ b/drivers/gpu/drm/xe/xe_macros.h
@@ -11,8 +11,8 @@
 #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))
+	({drm_dbg(&(xe)->drm, \
+		"Ioctl argument check failed at %s:%d: %s", \
+		__FILE__, __LINE__, #cond); (cond); })
 
 #endif
-- 
2.34.1

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

Posted by Lucas De Marchi 1 year, 3 months ago

On Sun, Oct 27, 2024 at 01:57:52PM +0900, Gyeyoung Baek wrote:
>In the previous code, there is build error.
>if CONFIG_DRM_USE_DYNAMIC_DEBUG is set,
>'drm_dbg' function is replaced with '__dynamic_func_call_cls',
>which is replaced with a do while statement.
>
>The problem is that,
>XE_IOCTL_DBG uses this function for conditional expression.
>
>so I fix the expression to be compatible with the do while statement,
>by referring to "https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html".
>
>Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
>---
> drivers/gpu/drm/xe/xe_macros.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_macros.h b/drivers/gpu/drm/xe/xe_macros.h
>index daf56c846d03..58a9d1e33502 100644
>--- a/drivers/gpu/drm/xe/xe_macros.h
>+++ b/drivers/gpu/drm/xe/xe_macros.h
>@@ -11,8 +11,8 @@
> #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))
>+	({drm_dbg(&(xe)->drm, \
>+		"Ioctl argument check failed at %s:%d: %s", \
>+		__FILE__, __LINE__, #cond); (cond); })

but this would print the debug message regardless of the cond being
true. Previously this would enter the condition if cond && 1 (due to the
comma operator use), but printing the message was shortcut when cond was
false.

It looks like keeping cond outside and the statement expr to cover only
the call to drm_dbg would work.

Lucas De Marchi

> #endif
>-- 
>2.34.1
>

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

Posted by gyeyoung 1 year, 3 months ago

Thank you for your review, I missed how && work.
I will revise a patch that print only when cond is true.

sincerely,
Gyeyoung baek

On Tue, Oct 29, 2024 at 4:47 AM Lucas De Marchi
<lucas.demarchi@intel.com> wrote:
>
> On Sun, Oct 27, 2024 at 01:57:52PM +0900, Gyeyoung Baek wrote:
> >In the previous code, there is build error.
> >if CONFIG_DRM_USE_DYNAMIC_DEBUG is set,
> >'drm_dbg' function is replaced with '__dynamic_func_call_cls',
> >which is replaced with a do while statement.
> >
> >The problem is that,
> >XE_IOCTL_DBG uses this function for conditional expression.
> >
> >so I fix the expression to be compatible with the do while statement,
> >by referring to "https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html".
> >
> >Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
> >---
> > drivers/gpu/drm/xe/xe_macros.h | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/xe/xe_macros.h b/drivers/gpu/drm/xe/xe_macros.h
> >index daf56c846d03..58a9d1e33502 100644
> >--- a/drivers/gpu/drm/xe/xe_macros.h
> >+++ b/drivers/gpu/drm/xe/xe_macros.h
> >@@ -11,8 +11,8 @@
> > #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))
> >+      ({drm_dbg(&(xe)->drm, \
> >+              "Ioctl argument check failed at %s:%d: %s", \
> >+              __FILE__, __LINE__, #cond); (cond); })
>
> but this would print the debug message regardless of the cond being
> true. Previously this would enter the condition if cond && 1 (due to the
> comma operator use), but printing the message was shortcut when cond was
> false.
>
> It looks like keeping cond outside and the statement expr to cover only
> the call to drm_dbg would work.
>
> Lucas De Marchi
>
> > #endif
> >--
> >2.34.1
> >