Re: Can smatch handle this better? (was: [PATCH 15/17] media: i2c: adp1653: introduce 'no_child' label)

[Dan Carpenter <error27@gmail.com>]

On Fri, Jan 27, 2023 at 08:43:51AM +0100, Hans Verkuil wrote:
> Hi Dan,
> 
> While trying to clean up smatch warnings in the media subsystem I came
> across a number of 'warn: missing unwind goto?' warnings that all had
> the same root cause as illustrated by this patch: the 'unwind' path
> just has a variation on printk(), it is not actually cleaning up anything.
> 
> As Sakari suggested, is this something that smatch can be improved for?
> These false positives are a bit annoying.
> 
> You can see the whole series here if you are interested:
> 
> https://patchwork.linuxtv.org/project/linux-media/list/?series=9747
> 
> Regards,
> 

Oh wow.  I really hate do nothing gotos.  The canonical bug for do
nothing gotos is forgetting to set the error code.

I like that check because it finds a lot of error handling bugs.

It's not just about the printk().  I could and probably should make the
check ignore printks.  There is also an of_node_put(child);

The check doesn't look at what the error handling does, only that there
is a direct returns surrounded by gotos.

I could make the check so that it's only enabled when --spammy is turned
on.

I guess another option would be to only enable the warning if there were
more than one label in the cleanup section at the end of the function.
I can make that a warning and if there is only one label, then make that
disable unless --spammy is used.

regards,
dan carpenter