Re: [linux-safety] [PATCH] usb: host: ehci-sched: add comment about find_tt() not returning error

From: Alan Stern
Date: Mon Oct 12 2020 - 12:00:16 EST


On Mon, Oct 12, 2020 at 05:10:21PM +0200, Lukas Bulwahn wrote:
>
>
> On Mon, 12 Oct 2020, Alan Stern wrote:
> > Real code contains so many assumptions, especially if you include ones
> > which are obvious to everybody, that such a tool seems impractical.
> >
>
> I fear that problem applies to all static code analysis tools I have seen;
> at some point, the remaining findings are simply obviously wrong to
> everybody but the tool does not get those assumptions and continues
> complaining, making the tool seem impractical.

Indeed, it is well known that the problem of finding all errors or bugs
by static code analysis is Turing complete.

> Alan, so would you be willing to take patches where _anyone_ simply adds
> comments on what functions returns, depending on what this person might
> consider just not obvious enough?

No. I would take such patches from anyone, but depending on what _I_
consider not obvious enough.

> Or are you going to simply reject this 'added a comment' patch here?

I have already accepted it. In fact, the patch was my suggestion in the
first place.

When I originally wrote this code, I was aware that it was somewhat
subtle, but at the time it didn't seem to warrant a comment or
explanation. Sudip's patch has changed my mind.

> I am not arguing either way, it is just that it is unclear to me what the
> added value of the comment really is here.

As with many other comments, its purpose is to explain a somewhat
obscure aspect of the code -- something which is there by design but
isn't immediately obvious to the reader. That is the added value.

> And for the static analysis finding, we need to find a way to ignore this
> finding without simply ignoring all findings or new findings that just
> look very similar to the original finding, but which are valid.

Agreed. In this case, the new comment does a pretty good job of telling
people using the tool that the finding is unjustified.

If you are suggesting some sort of special code annotation that the tool
would understand, I am open to that. But I'm not aware of any even
vaguely standard way of marking up a particular function call to
indicate it will not return an error.

Alan Stern