lkml.org 
[lkml]   [2017]   [Oct]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] sr9800: Use common error handling code in sr9800_phy_powerup()
Hi Markus,

On Sun, Oct 29, 2017 at 1:03 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
>>> @@ -700,10 +700,9 @@ static int sr9800_phy_powerup(struct usbnet *dev)
>>>
>>> /* set the embedded Ethernet PHY in power-up state */
>>> ret = sr_sw_reset(dev, SR_SWRESET_IPRL);
>>> - if (ret < 0) {
>>> - netdev_err(dev->net, "Failed to reset PHY: %d\n", ret);
>>> - return ret;
>>> - }
>>> + if (ret < 0)
>>> + goto report_reset_failure;
>>
>> So now I have to look below to see what error handling it does...
>
> Yes. - Can this be an usual consequence if you apply information from
> the section “7) Centralized exiting of functions” in the document
> “coding-style.rst” a bit more?

That section is useful if several cleanups steps have to be performed.
In this case, the only common part is the printing of the error message.
Printing error messages is not a cleanup step.

What's also bad in this case, is that after the first "goto", there are other
error cases that don't use goto, but just return, because there's no cleanup
to do there.

Hence:
NAKed-by: Geert Uytterhoeven <geert@linux-m68k.org>

>> Hence I prefer the original version, which had _less_ lines of code...
>
> My update suggestion is only one line “bigger” in this case, isn't it?
>
> I propose an other source code layout so that a bit smaller executable
> object code could be achieved.
> Do find such a software design direction feasible?

If you play the "smaller executable object code" card, people expect that
you provide the actual numbers, too.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

\
 
 \ /
  Last update: 2017-10-30 11:00    [W:0.043 / U:0.552 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site