Am 20.12.2017 11:30, schrieb Julia Lawall: > > > On Wed, 20 Dec 2017, Dan Carpenter wrote: > >> On Tue, Dec 19, 2017 at 10:59:52PM +0200, Andy Shevchenko wrote: >>> @@ -914,9 +904,7 @@ static int lm3554_probe(struct i2c_client *client) >>> dev_err(&client->dev, "gpio request/direction_output fail"); >>> goto fail2; >>> } >>> - if (ACPI_HANDLE(&client->dev)) >>> - err = atomisp_register_i2c_module(&flash->sd, NULL, LED_FLASH); >>> - return 0; >>> + return atomisp_register_i2c_module(&flash->sd, NULL, LED_FLASH); >>> fail2: >>> media_entity_cleanup(&flash->sd.entity); >>> v4l2_ctrl_handler_free(&flash->ctrl_handler); >> >> Actually every place where we directly return a function call is wrong >> and needs error handling added. I've been meaning to write a Smatch >> check for this because it's a common anti-pattern we don't check the >> last function call for errors. >> >> Someone could probably do the same in Coccinelle if they want. > > I'm not sure what you are suggesting. Is every case of return f(...); > for any f wrong? Or is it a particular function that is of concern? Or > would it be that every function call that has error handling somewhere > should have error handling everywhere? Or is it related to what seems to > be the problem in the above code that err is initialized but nothing > happens to it? > I guess the idea is to check if a return value gets set and then discarded because the function returns const. IMHO this is a case of write-never read like that series what Colin King fixed lately. re, wh -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html