Re: [PATCH v1 05/10] staging: atomisp: Remove non-ACPI leftovers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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



[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux