archive mirror
 help / color / mirror / Atom feed
From: Dan Carpenter <>
To: Laurent Pinchart <>
Cc: Tomi Valkeinen <>,
	Kieran Bingham <>,
	Mauro Carvalho Chehab <>,
	Hans Verkuil <>,
	Sakari Ailus <>,,
Subject: Re: [PATCH] media: v4l2-subdev: fix some NULL vs IS_ERR() checks
Date: Tue, 22 Jun 2021 18:58:58 +0300	[thread overview]
Message-ID: <20210622155858.GN1861@kadam> (raw)
In-Reply-To: <>

On Tue, Jun 22, 2021 at 06:08:30PM +0300, Laurent Pinchart wrote:
> Hi Dan,
> Thank you for the patch.
> On Tue, Jun 22, 2021 at 05:31:53PM +0300, Dan Carpenter wrote:
> > The v4l2_subdev_alloc_state() function returns error pointers, it
> > doesn't return NULL.
> It's funny you send this patch today, I've been thinking about the exact
> same issue yesterday, albeit more globally, when trying to figure out if
> a function I called returned NULL or an error pointer on error.
> Would it make to create an __err_ptr annotation to mark functions that
> return an error pointer ? This would both give a simple indication to
> the user and allow tools such as smatch to detect errors.

If you have the cross function DB enabled then Smatch can figure out if
it returns error pointers or NULL.  The big problem is that Smatch works
on the precompiled code and doesn't understand ifdeffed code.

I haven't pushed all the Smatch checks.  I told someone last month, I'd
give them a month to fix any bugs since it was their idea.  But I'll
push it soon.

function returns error pointer or valid
struct foo *function() { return NULL; }

I believe that there are also people who use a two pass Coccinelle
system where they make a list of functions that return error pointers
and then check the callers.

The Huawei devs find a bunch of these bugs through static analysis but
I don't know which tools they are using.

Today, I accidentally introduced a bug by converting a call that can
"in theory/the future return error pointers" but also returns NULL at
the end of a list.  I thought it was only supposed to be checked for
NULLs.  Fortunately Colin King found it right away.  That was just
sloppiness on my part :/ and it's pretty rare to find code like that.

dan carpenter

  reply	other threads:[~2021-06-22 15:59 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-22 14:31 Dan Carpenter
2021-06-22 15:08 ` Laurent Pinchart
2021-06-22 15:58   ` Dan Carpenter [this message]
2021-06-23  2:29     ` weiyongjun (A)
2021-06-23  2:34     ` Laurent Pinchart
2021-06-23  9:03       ` Dan Carpenter
2021-06-23 12:56         ` Laurent Pinchart
2021-06-22 20:01 ` Sakari Ailus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210622155858.GN1861@kadam \ \ \ \ \ \ \ \ \ \
    --subject='Re: [PATCH] media: v4l2-subdev: fix some NULL vs IS_ERR() checks' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).