linux-kernel - Re: [RFC PATCH RESEND] tracing: add kernel documentation for trace_array_set_clr_event, trace_set_clr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+wEVJab6Gwkd7q58=v8uVhfmKpwNRAqtK67f15JtZcKSB_ziA@mail.gmail.com>
Date: Wed, 9 Jul 2025 15:35:50 +0200
From: Gabriele Paoloni <gpaoloni@...hat.com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: "Masami Hiramatsu (Google)" <mhiramat@...nel.org>, mathieu.desnoyers@...icios.com, 
	linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org
Subject: Re: [RFC PATCH RESEND] tracing: add kernel documentation for
 trace_array_set_clr_event, trace_set_clr_event and supporting functions

[resending as my previous reply had HTML stuff and was rejected]
Apologies for duplicates

On Wed, Jul 9, 2025 at 3:25 AM Steven Rostedt <rostedt@...dmis.org> wrote:
>
> On Wed, 9 Jul 2025 10:06:26 +0900
> Masami Hiramatsu (Google) <mhiramat@...nel.org> wrote:
>
> > On Tue, 1 Jul 2025 19:59:39 -0400
> > Steven Rostedt <rostedt@...dmis.org> wrote:
> >
> > > > + * __ftrace_event_enable_disable - enable or disable a trace event
> > > > + * @file: trace event file associated with the event.
> > > > + * @enable: 0 or 1 respectively to disable/enable the event (any other value is
> > > > + * invalid).
> > >
> > > Saying 0 or 1 should assume that those are the only values. Don't need the
> > > content in the parenthesis.
> >
> > BTW, it should be "0 or !0"? (or we should make it boolean)
> > This description means if it is "2", that is undefined.
>
> 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...).

>
> 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....

Thanks
Gab

>
> -- Steve
>