Re: [PATCH v2] cxl/trace: Properly initialize cxl_poison region name

[Alison Schofield <alison.schofield@intel.com>]

On Thu, Mar 14, 2024 at 05:17:37PM -0400, Steven Rostedt wrote:
> On Thu, 14 Mar 2024 14:05:00 -0700
> Alison Schofield <alison.schofield@intel.com> wrote:
> 
> > > I'm still curious to why NULL didn't work. I guess it may never have as I
> > > noticed there's nothing else doing that. There are cases that a variable
> > > returns NULL and the __string() handles it. But I guess the compiler gets
> > > confused if the NULL is a possible return to the condition in __string().  
> > 
> > Here's the full warning spew:
> > 
> > In file included from ./include/trace/define_trace.h:102,
> >                  from drivers/cxl/core/trace.h:713,
> >                  from drivers/cxl/core/trace.c:8:
> > drivers/cxl/core/./trace.h: In function ‘trace_event_get_offsets_cxl_poison’:
> > ./include/trace/stages/stage5_get_offsets.h:50:21: warning: argument 1 null where non-null expected [-Wnonnull]
> >    50 |                     strlen((src) ? (const char *)(src) : "(null)") + 1)
> >       |                     ^~~~~~
> > ./include/trace/trace_events.h:263:9: n
> 
> The full warning didn't add any new information. The above was all I
> needed. But I'm curious if you applied this patch if the warning will go
> away. (Note I didn't test nor compile this).
> 
> -- Steve
> 
> diff --git a/include/trace/stages/stage5_get_offsets.h b/include/trace/stages/stage5_get_offsets.h
> index e30a13be46ba..dfae18d8f4df 100644
> --- a/include/trace/stages/stage5_get_offsets.h
> +++ b/include/trace/stages/stage5_get_offsets.h
> @@ -9,6 +9,13 @@
>  #undef __entry
>  #define __entry entry
>  

+#ifndef STAGE5_H_INCLUDED
+#define STAGE5_H_INCLUDED
> +static inline const char *__string_src(const char *str)
> +{
> +	if (!str)
> +		return "(null)";
> +	return str;
> +}
+#endif /* STAGE5_H_INCLUDED */

Happy to try it out...

Your diff, with the ifdef above, makes the compiler happy and it functions
as wanted - no garbage in the fields.

> +
>  /*
>   * Fields should never declare an array: i.e. __field(int, arr[5])
>   * If they do, it will cause issues in parsing and possibly corrupt the
> @@ -47,7 +54,7 @@
>  
>  #undef __string
>  #define __string(item, src) __dynamic_array(char, item,			\
> -		    strlen((src) ? (const char *)(src) : "(null)") + 1)
> +		    strlen(__string_src(src)) + 1)
>  
>  #undef __string_len
>  #define __string_len(item, src, len) __dynamic_array(char, item, (len) + 1)