On 27/01/2023 13:41, Dan Carpenter wrote: > 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. I do too, I did find some real bugs with this as well. > > 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); This are other examples in my patch series where there is only an error message: https://patchwork.linuxtv.org/project/linux-media/patch/20230126150657.367921-15-hverkuil-cisco@xxxxxxxxx/ https://patchwork.linuxtv.org/project/linux-media/patch/20230126150657.367921-17-hverkuil-cisco@xxxxxxxxx/ > > 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. That would cause us to miss a real bug like this: https://patchwork.linuxtv.org/project/linux-media/patch/20230126150657.367921-8-hverkuil-cisco@xxxxxxxxx/ I think that if you were able to check that all the code after a goto label consisted of variations on printk, then that would solve most of the false positives I've seen in the media subsystem. The few remaining ones we can work around ourselves. Regards, Hans > > regards, > dan carpenter >
