Re: [PATCH] [PATCH] cxl: trace: Fix macro safety in CXL_EVT_TP_fast_assign

[Ira Weiny <ira.weiny@intel.com>]

shaikh.kamal wrote:
> Fix checkpatch.pl detected error
> The CXL_EVT_TP_fast_assign macro assigns multiple fields, but does not
> wrap the body in a `do { ... } while (0)` block. This can lead to
> unexpected behavior when used in conditional branches.
> 
> Add checks to ensure cxlmd is valid before accessing its fields.
> 
> Signed-off-by: shaikh.kamal <shaikhkamal2012@gmail.com>

Nak

This is not a normal macro.  checkpatch.pl does not apply in this case.
Also checking cxlmd here is not necessary.  The callers of the tracepoint
know very well that it needs to be valid.

Ira

> ---
>  drivers/cxl/core/trace.h | 30 ++++++++++++++++++------------
>  1 file changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
> index 25ebfbc1616c..a1a1014ee1fe 100644
> --- a/drivers/cxl/core/trace.h
> +++ b/drivers/cxl/core/trace.h
> @@ -249,18 +249,24 @@ TRACE_EVENT(cxl_overflow,
>  	__field(u8, hdr_maint_op_class)				\
>  	__field(u8, hdr_maint_op_sub_class)
>  
> -#define CXL_EVT_TP_fast_assign(cxlmd, l, hdr)					\
> -	__assign_str(memdev);				\
> -	__assign_str(host);			\
> -	__entry->log = (l);							\
> -	__entry->serial = (cxlmd)->cxlds->serial;				\
> -	__entry->hdr_length = (hdr).length;					\
> -	__entry->hdr_flags = get_unaligned_le24((hdr).flags);			\
> -	__entry->hdr_handle = le16_to_cpu((hdr).handle);			\
> -	__entry->hdr_related_handle = le16_to_cpu((hdr).related_handle);	\
> -	__entry->hdr_timestamp = le64_to_cpu((hdr).timestamp);			\
> -	__entry->hdr_maint_op_class = (hdr).maint_op_class;			\
> -	__entry->hdr_maint_op_sub_class = (hdr).maint_op_sub_class
> +#define CXL_EVT_TP_fast_assign(cxlmd, l, hdr) \
> +	do { \
> +		if (!(cxlmd)) { \
> +			pr_err("Invalid arguments to CXL_EVT_TP_fast_assign\n"); \
> +			break; \
> +		} \
> +		__assign_str(memdev); \
> +		__assign_str(host); \
> +		__entry->log = (l); \
> +		__entry->serial = (cxlmd)->cxlds->serial; \
> +		__entry->hdr_length = (hdr).length; \
> +		__entry->hdr_flags = get_unaligned_le24((hdr).flags); \
> +		__entry->hdr_handle = le16_to_cpu((hdr).handle); \
> +		__entry->hdr_related_handle = le16_to_cpu((hdr).related_handle); \
> +		__entry->hdr_timestamp = le64_to_cpu((hdr).timestamp); \
> +		__entry->hdr_maint_op_class = (hdr).maint_op_class; \
> +		__entry->hdr_maint_op_sub_class = (hdr).maint_op_sub_class; \
> +	} while (0)
>  
>  #define CXL_EVT_TP_printk(fmt, ...) \
>  	TP_printk("memdev=%s host=%s serial=%lld log=%s : time=%llu uuid=%pUb "	\
> -- 
> 2.43.0
>