Re: [PATCH] media: v4l2-subdev: fix some NULL vs IS_ERR() checks

Laurent Pinchart
2021-06-23T12:56:42Z

On Wed, Jun 23, 2021 at 12:03:26PM +0300, Dan Carpenter wrote:
> On Wed, Jun 23, 2021 at 05:34:16AM +0300, Laurent Pinchart wrote:
> >
> > Do you think an annotation could still help, by making it explicit in
> > headers whether a function returns NULL or an error pointer, thus
> > helping developers get it right in the first place ?
>
> Not really. It wouldn't help with Smatch. I really think error pointer
> bugs are handled pretty well currently. Sometimes I have seen syzbot
> find them before the static checkers but I don't see them affecting
> users and production kernels.

I meant to ask if it would be useful for developers, not for smatch.
When I use a function and have to figure out whether to use IS_ERR() or
!= NULL, I first look at the header, and most of the time I then need to
find the corresponding implementation, wherever it may be. If we had an
annotation, the second step could be skipped. Of course the annotation
would need to match the implementation, and that's an area where smatch
could help.

> There are few other things that Smatch looks for like passing positives,
> valid pointers or NULLs to PTR_ERR(). I do wish that when functions
> return a mix of negative error codes, 0 and 1 that they had comment
> explaining what the 1 means.

--
Regards,

Laurent Pinchart