Re: [RFC PATCH RESEND] tracing: add kernel documentation for trace_array_set_clr_event, trace_set_clr_event and supporting functions

[Steven Rostedt <rostedt@goodmis.org>]

On Wed, 9 Jul 2025 15:35:50 +0200
Gabriele Paoloni <gpaoloni@redhat.com> wrote:

> > Hmm, now here's an interesting point. So this is to define requirements
> > of a function based on what the function is doing. But does the
> > function have to have strict requirements?  
> 
> IMO one of the main goals for these requirements is testability.
> In order to have testable requirements we should state what the
> valid input values are. In this case:
> 0 -> disable, 1 -> enable, everything else -> Error.
> 
> Now checking the code again it seems that the switch statement
> is missing a default "ret = -EINVAL" (or else it should be changed
> to boolean, but I guess it would have a wider impact on the rest
> of the code...).

Well, it's mostly used internally and the only places that call it uses
0 or 1, so there's never been any issue.

> 
> >
> > If it can handle "0" or "!0" does that mean that's how it will be
> > defined? Or can it just state "0" or "1" and yes "2" is UB. That is,
> > it's not part of the requirements but if someone passes in 2, it could
> > act as a 1 as it's UB and implementation defined. Not a requirement.  
> 
> Right now if 2 is passed the function would silently return success,
> but do we have a use case for this? I am trying to understand
> where the implementation defined behavior would be....

The issue is that all the callers pass in the proper value, and that
can be easily verified, but by adding the "anything else ERROR", it
would require adding more code that is not needed.

I rather just switch that and soft_disable into a boolean than to add
superficial error checks.

-- Steve