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, Hans On 26/01/2023 16:19, Sakari Ailus wrote: > Hi Hans, > > On Thu, Jan 26, 2023 at 04:06:55PM +0100, Hans Verkuil wrote: >> The code mixed gotos and returns, which confused smatch. Add a no_child >> label before the 'return -EINVAL;' to help smatch understand this. >> It's a bit more consistent as well. >> >> This fixes this smatch warning: >> >> adp1653.c:444 adp1653_of_init() warn: missing unwind goto? > > This is clearly a false positive. There's also no reason to just have a > label where you simply return a value. > > Would it be possible to just fix smatch instead? > >> >> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx> >> Cc: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> >> --- >> drivers/media/i2c/adp1653.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/media/i2c/adp1653.c b/drivers/media/i2c/adp1653.c >> index a61a77de6eee..136bca801db7 100644 >> --- a/drivers/media/i2c/adp1653.c >> +++ b/drivers/media/i2c/adp1653.c >> @@ -420,7 +420,7 @@ static int adp1653_of_init(struct i2c_client *client, >> >> child = of_get_child_by_name(node, "flash"); >> if (!child) >> - return -EINVAL; >> + goto no_child; >> >> if (of_property_read_u32(child, "flash-timeout-us", >> &pd->max_flash_timeout)) >> @@ -441,7 +441,7 @@ static int adp1653_of_init(struct i2c_client *client, >> >> child = of_get_child_by_name(node, "indicator"); >> if (!child) >> - return -EINVAL; >> + goto no_child; >> >> if (of_property_read_u32(child, "led-max-microamp", >> &pd->max_indicator_intensity)) >> @@ -459,6 +459,7 @@ static int adp1653_of_init(struct i2c_client *client, >> err: >> dev_err(&client->dev, "Required property not found\n"); >> of_node_put(child); >> +no_child: >> return -EINVAL; >> } >> >
