Re: [PATCH] sr9800: Use common error handling code in sr9800_phy_powerup()

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

 



Hi Markus,

On Sun, Oct 29, 2017 at 1:03 PM, SF Markus Elfring
<elfring@xxxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>

>> 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@xxxxxxxxxxxxxx

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
--
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