On 2/2/22 10:47 AM, Damien Le Moal wrote: >> ata_host_suspend() always returns 0, so the result checks in the drivers >> are pointless. However I chose not to change ata_host_suspend() to *void* >> not to have to change >> >> return ata_host_suspend(...); >> to >> ata_host_suspend(...); >> return 0; > > Nice cleanup, I like it. But given how simple ata_host_suspend() is, I > would prefer that it is turned into a void function... > > My view is: if the function returns an int error code, it should be > checked, always. If said function always return "success", then it > should be void. This makes it clear for the users (the different > drivers) how the function should be used. > > With your change, anybody looking at the driver code and at > ata_host_suspend() interface in the header file may think "the error > check is missing", unless the person also look at the function code... This isseu largely eavdes me as I use Emacs' support of the TAGS file. :-) > So let's simplify and make ata_host_suspend() a void function. There are > not that many call sites to change. Not all users do "return > ata_host_suspend()". OK, I've prepared 2 variants of this patch in anticipation of your reaction -- will just post the 2nd one. :-) >> Found by Linux Verification Center (linuxtesting.org) with the SVACE static >> analysis tool. >> >> Signed-off-by: Sergey Shtylyov <s.shtylyov@xxxxxx> [...] MBR, Sergey
