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

From: shaikh kamaluddin
Date: Thu Apr 24 2025 - 11:47:48 EST



On 4/22/25 00:42, Steven Rostedt wrote:
On Tue, 22 Apr 2025 00:15:17 +0530
"shaikh.kamal" <shaikhkamal2012@xxxxxxxxx> wrote:

Fix checkpatch.pl detected error
First, checkpatch.pl should never be used on existing code unless it's
yours. As the name suggests, it's for checking patches. It's not to check
what's already been accepted. Please do not submit patches against accepted
code because of what checkpatch.pl reports.
Thanks for your suggestion.

If you run it on code and it reports something that you find is a real bug,
then sure, fix it. But don't submit patches on code you do not understand
just because checkpatch.pl says there's an issue with it.
Okay got it.

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.
Next, this is not a normal macro. It's a trace event macro and
checkpatch.pl fails miserably on pretty much all tracing macros.

Thanks for your comments.

Add checks to ensure cxlmd is valid before accessing its fields.
If it is invalid, and we do what your patch suggests and just not write
anything, the event it was writing into has already been created. If we
exit out of this macro, then the event will simply contain garbage, but is
already on its way to user space. That means the output to user space will
be garbage. That's a bug. In other words, if cxlmd is NULL, it had better
not be calling this macro in the fist place!

-- Steve
Thanks for your suggestions and comments.