Re: [PATCH 5/7] staging: ccree: add clock management support

From: Gilad Ben-Yossef
Date: Thu Jun 22 2017 - 09:29:14 EST


On Thu, Jun 22, 2017 at 11:58 AM, Dan Carpenter
<dan.carpenter@xxxxxxxxxx> wrote:
> On Thu, Jun 22, 2017 at 10:07:51AM +0300, Gilad Ben-Yossef wrote:
>> +int cc_clk_on(struct ssi_drvdata *drvdata)
>> +{
>> + int rc = 0;
>> + struct clk *clk = drvdata->clk;
>> +
>> + if (IS_ERR(clk))
>> + /* No all devices have a clock associated with CCREE */
>> + goto out;
>
> Ugh... I hate this. The "goto out;" here is a waste of time
> do-nothing-goto that returns diretly. It's equivalent to "return 0;".
> Is that intended? Even with the comment, it's not clear...
>
> People think do nothing gotos are a great idea but from reviewing tons
> and tons of real life errors, I can assure you that in real life (as
> opposed to theory) they don't prevent any future bugs and only introduce
> "forgot to set the error code" bugs.

I see your point and the code is indeed clearer this way.

Will fix.

Thanks,
Gilad


--
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru