Re: [PATCH v1] rtla: Fix implicit NULL dereference

[Dan Carpenter <dan.carpenter@linaro.org>]

The subject is bad because it says "Fix" when this is a clean up
and it says "NULL dereference" when there isn't any NULL dereference.

On Thu, Jan 09, 2025 at 11:13:26PM +0200, Costa Shulyupin wrote:
> The `record` variable is NULL when tracing is not requested:
> 
> struct osnoise_tool *record = NULL;
> 
> if (params->trace_output) {
>         record = osnoise_init_trace_tool("osnoise");
> ....
> 
> Value of `&record->trace` in this case is NULL just because
> the `trace` member is the first member `struct osnoise_tool` with offset 0.
> `&record->trace` just returns the offset.
> 
> Explicit dereference `record->trace' would cause segmentation fault.
> 
> Add explicit check for zero `record`.
> 

This commit message is very confusing.  I would normally not send a
patch like this, but if I did send it the commit message would say
something like:

  The "record" pointer can be NULL in this code.  When we're calling
  trace_is_off(&tool->trace, &record->trace) and "record" is NULL then
  it kind of looks like a NULL dereference.  It turns out that it's
  fine when you look at it more closely, but at first glance it looks
  sketchy.  Add an explicit NULL check to make the code more clear.

Tracing code is generally fast path code so maybe we don't want to add
a NULL check?  If we were really bothered by the existing code then a
better fix would be to add an inline function to do it.

regards,
dan carpenter