On Wed, Jul 9, 2025 at 4:12 PM Steven Rostedt <rost...@goodmis.org> wrote: > > On Wed, 9 Jul 2025 15:35:50 +0200 > Gabriele Paoloni <gpaol...@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.
Many thanks for the explanation. Please consider that, from a requirement point of view, it is also perfectly fine to document the assumption of use that no other values than 0 or 1 shall be passed by the caller (so the bool rework can be avoided eventually). Gab > > > -- Steve >
