| Message ID | 38279cb8-ff60-427e-ae9f-5f973955ffa6@web.de (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2] net: bcmasp: Use common error handling code in bcmasp_probe() | expand |
On Wed, Nov 15, 2023 at 1:11 AM Markus Elfring <Markus.Elfring@web.de> wrote: > > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Wed, 15 Nov 2023 09:38:56 +0100 > > Add a jump target so that a bit of exception handling can be better reused > in this function implementation. > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > --- > > v2: > Special expectations were expressed for the previous patch size. > > * Jakub Kicinski > https://lore.kernel.org/netdev/20231106145806.669875f4@kernel.org/ > > * Justin Chen > https://lore.kernel.org/netdev/CALSSxFYRgPwEq+QhCOYPqrtae8RvL=jTOcz4mk3vbe+Fc0QwbQ@mail.gmail.com/ > > * Florian Fainelli > https://lore.kernel.org/netdev/4053e838-e5cf-4450-8067-21bdec989d1b@broadcom.com/ > > > Thus another change variant can eventually be integrated. > > > drivers/net/ethernet/broadcom/asp2/bcmasp.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/broadcom/asp2/bcmasp.c b/drivers/net/ethernet/broadcom/asp2/bcmasp.c > index 29b04a274d07..7d6c15732d9f 100644 > --- a/drivers/net/ethernet/broadcom/asp2/bcmasp.c > +++ b/drivers/net/ethernet/broadcom/asp2/bcmasp.c > @@ -1304,9 +1304,8 @@ static int bcmasp_probe(struct platform_device *pdev) > intf = bcmasp_interface_create(priv, intf_node, i); > if (!intf) { > dev_err(dev, "Cannot create eth interface %d\n", i); > - bcmasp_remove_intfs(priv); > of_node_put(intf_node); > - goto of_put_exit; > + goto remove_intfs; > } > list_add_tail(&intf->list, &priv->intfs); > i++; > @@ -1331,6 +1330,7 @@ static int bcmasp_probe(struct platform_device *pdev) > netdev_err(intf->ndev, > "failed to register net_device: %d\n", ret); > priv->destroy_wol(priv); > +remove_intfs: > bcmasp_remove_intfs(priv); > goto of_put_exit; > } > -- > 2.42.1 > nak. Doesn't save any lines of code. Doesn't make things clearer or easier to follow. This doesn't seem like an improvement to me. Justin
>> Add a jump target so that a bit of exception handling can be better reused >> in this function implementation. … >> +++ b/drivers/net/ethernet/broadcom/asp2/bcmasp.c >> @@ -1304,9 +1304,8 @@ static int bcmasp_probe(struct platform_device *pdev) >> intf = bcmasp_interface_create(priv, intf_node, i); >> if (!intf) { >> dev_err(dev, "Cannot create eth interface %d\n", i); >> - bcmasp_remove_intfs(priv); >> of_node_put(intf_node); >> - goto of_put_exit; >> + goto remove_intfs; >> } >> list_add_tail(&intf->list, &priv->intfs); >> i++; >> @@ -1331,6 +1330,7 @@ static int bcmasp_probe(struct platform_device *pdev) >> netdev_err(intf->ndev, >> "failed to register net_device: %d\n", ret); >> priv->destroy_wol(priv); >> +remove_intfs: >> bcmasp_remove_intfs(priv); >> goto of_put_exit; >> } >> -- >> 2.42.1 >> > nak. Doesn't save any lines of code. Would you get into the mood to take also another look at consequences for executable code? > Doesn't make things clearer or easier to follow. Maybe. > This doesn't seem like an improvement to me. Can this software implementation benefit from one function call less? Regards, Markus
diff --git a/drivers/net/ethernet/broadcom/asp2/bcmasp.c b/drivers/net/ethernet/broadcom/asp2/bcmasp.c index 29b04a274d07..7d6c15732d9f 100644 --- a/drivers/net/ethernet/broadcom/asp2/bcmasp.c +++ b/drivers/net/ethernet/broadcom/asp2/bcmasp.c @@ -1304,9 +1304,8 @@ static int bcmasp_probe(struct platform_device *pdev) intf = bcmasp_interface_create(priv, intf_node, i); if (!intf) { dev_err(dev, "Cannot create eth interface %d\n", i); - bcmasp_remove_intfs(priv); of_node_put(intf_node); - goto of_put_exit; + goto remove_intfs; } list_add_tail(&intf->list, &priv->intfs); i++; @@ -1331,6 +1330,7 @@ static int bcmasp_probe(struct platform_device *pdev) netdev_err(intf->ndev, "failed to register net_device: %d\n", ret); priv->destroy_wol(priv); +remove_intfs: bcmasp_remove_intfs(priv); goto of_put_exit; }