[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAOtvUMdrb+KuEAYHF9jpqxtYcjZ=351L-ZQAWL3CU6KVhvgMWQ@mail.gmail.com>
Date: Thu, 22 Jun 2017 16:29:01 +0300
From: Gilad Ben-Yossef <gilad@...yossef.com>
To: Dan Carpenter <dan.carpenter@...cle.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Linux kernel mailing list <linux-kernel@...r.kernel.org>,
linux-crypto@...r.kernel.org,
driverdev-devel@...uxdriverproject.org, devel@...verdev.osuosl.org,
Ofir Drang <ofir.drang@....com>
Subject: Re: [PATCH 5/7] staging: ccree: add clock management support
On Thu, Jun 22, 2017 at 11:58 AM, Dan Carpenter
<dan.carpenter@...cle.com> 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
Powered by blists - more mailing lists