* [PATCH] usb/peak_usb: cleanup code
@ 2022-05-11 6:38 Bernard Zhao
2022-05-11 6:44 ` Marc Kleine-Budde
0 siblings, 1 reply; 6+ messages in thread
From: Bernard Zhao @ 2022-05-11 6:38 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Jakub Kicinski, Paolo Abeni, Stefan Mätje, Vincent Mailhol,
Bernard Zhao, linux-can, netdev, linux-kernel
Cc: bernard
The variable fi and bi only used in branch if (!dev->prev_siblings)
, fi & bi not kmalloc in else branch, so move kfree into branch
if (!dev->prev_siblings),this change is to cleanup the code a bit.
Signed-off-by: Bernard Zhao <zhaojunkui2008@126.com>
---
drivers/net/can/usb/peak_usb/pcan_usb_pro.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
index ebe087f258e3..70c5aef57247 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
@@ -903,6 +903,9 @@ static int pcan_usb_pro_init(struct peak_usb_device *dev)
pcan_usb_pro.name,
bi->hw_rev, bi->serial_num_hi, bi->serial_num_lo,
pcan_usb_pro.ctrl_count);
+
+ kfree(bi);
+ kfree(fi);
} else {
usb_if = pcan_usb_pro_dev_if(dev->prev_siblings);
}
@@ -913,9 +916,6 @@ static int pcan_usb_pro_init(struct peak_usb_device *dev)
/* set LED in default state (end of init phase) */
pcan_usb_pro_set_led(dev, PCAN_USBPRO_LED_DEVICE, 1);
- kfree(bi);
- kfree(fi);
-
return 0;
err_out:
--
2.33.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] usb/peak_usb: cleanup code
2022-05-11 6:38 [PATCH] usb/peak_usb: cleanup code Bernard Zhao
@ 2022-05-11 6:44 ` Marc Kleine-Budde
2022-05-11 7:11 ` z
0 siblings, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2022-05-11 6:44 UTC (permalink / raw)
To: Bernard Zhao
Cc: Wolfgang Grandegger, David S. Miller, Jakub Kicinski,
Paolo Abeni, Stefan Mätje, Vincent Mailhol, linux-can,
netdev, linux-kernel, bernard
[-- Attachment #1: Type: text/plain, Size: 644 bytes --]
On 10.05.2022 23:38:38, Bernard Zhao wrote:
> The variable fi and bi only used in branch if (!dev->prev_siblings)
> , fi & bi not kmalloc in else branch, so move kfree into branch
> if (!dev->prev_siblings),this change is to cleanup the code a bit.
Please move the variable declaration into that scope, too. Adjust the
error handling accordingly.
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re:Re: [PATCH] usb/peak_usb: cleanup code
2022-05-11 6:44 ` Marc Kleine-Budde
@ 2022-05-11 7:11 ` z
2022-05-11 8:28 ` Vincent MAILHOL
0 siblings, 1 reply; 6+ messages in thread
From: z @ 2022-05-11 7:11 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Wolfgang Grandegger, David S. Miller, Jakub Kicinski,
Paolo Abeni, Stefan Mätje, Vincent Mailhol, linux-can,
netdev, linux-kernel, bernard
At 2022-05-11 14:44:50, "Marc Kleine-Budde" <mkl@pengutronix.de> wrote:
>On 10.05.2022 23:38:38, Bernard Zhao wrote:
>> The variable fi and bi only used in branch if (!dev->prev_siblings)
>> , fi & bi not kmalloc in else branch, so move kfree into branch
>> if (!dev->prev_siblings),this change is to cleanup the code a bit.
>
>Please move the variable declaration into that scope, too. Adjust the
>error handling accordingly.
Hi Marc:
I am not sure if there is some gap.
If we move the variable declaration into that scope, then each error branch has to do the kfree job, like:
if (err) {
dev_err(dev->netdev->dev.parent,
"unable to read %s firmware info (err %d)\n",
pcan_usb_pro.name, err);
kfree(bi);
kfree(fi);
kfree(usb_if);
return err;
}
I am not sure if this looks a little less clear?
Thanks!
BR//Bernard
>
>regards,
>Marc
>
>--
>Pengutronix e.K. | Marc Kleine-Budde |
>Embedded Linux | https://www.pengutronix.de |
>Vertretung West/Dortmund | Phone: +49-231-2826-924 |
>Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Re: [PATCH] usb/peak_usb: cleanup code
2022-05-11 7:11 ` z
@ 2022-05-11 8:28 ` Vincent MAILHOL
2022-05-11 8:46 ` z
2022-05-11 8:47 ` Marc Kleine-Budde
0 siblings, 2 replies; 6+ messages in thread
From: Vincent MAILHOL @ 2022-05-11 8:28 UTC (permalink / raw)
To: z
Cc: Marc Kleine-Budde, Wolfgang Grandegger, David S. Miller,
Jakub Kicinski, Paolo Abeni, Stefan Mätje, linux-can,
netdev, linux-kernel, bernard
On Wed. 11 May 2022 at 16:11, z <zhaojunkui2008@126.com> wrote:
> At 2022-05-11 14:44:50, "Marc Kleine-Budde" <mkl@pengutronix.de> wrote:
> >On 10.05.2022 23:38:38, Bernard Zhao wrote:
> >> The variable fi and bi only used in branch if (!dev->prev_siblings)
> >> , fi & bi not kmalloc in else branch, so move kfree into branch
> >> if (!dev->prev_siblings),this change is to cleanup the code a bit.
> >
> >Please move the variable declaration into that scope, too. Adjust the
> >error handling accordingly.
>
> Hi Marc:
>
> I am not sure if there is some gap.
> If we move the variable declaration into that scope, then each error branch has to do the kfree job, like:
> if (err) {
> dev_err(dev->netdev->dev.parent,
> "unable to read %s firmware info (err %d)\n",
> pcan_usb_pro.name, err);
> kfree(bi);
> kfree(fi);
> kfree(usb_if);
>
> return err;
> }
> I am not sure if this looks a little less clear?
> Thanks!
A cleaner way would be to move all the content of the if
(!dev->prev_siblings) to a new function.
Yours sincerely,
Vincent Mailhol
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re:Re: Re: [PATCH] usb/peak_usb: cleanup code
2022-05-11 8:28 ` Vincent MAILHOL
@ 2022-05-11 8:46 ` z
2022-05-11 8:47 ` Marc Kleine-Budde
1 sibling, 0 replies; 6+ messages in thread
From: z @ 2022-05-11 8:46 UTC (permalink / raw)
To: Vincent MAILHOL
Cc: Marc Kleine-Budde, Wolfgang Grandegger, David S. Miller,
Jakub Kicinski, Paolo Abeni, Stefan Mätje, linux-can,
netdev, linux-kernel, bernard
At 2022-05-11 16:28:26, "Vincent MAILHOL" <mailhol.vincent@wanadoo.fr> wrote:
>On Wed. 11 May 2022 at 16:11, z <zhaojunkui2008@126.com> wrote:
>> At 2022-05-11 14:44:50, "Marc Kleine-Budde" <mkl@pengutronix.de> wrote:
>> >On 10.05.2022 23:38:38, Bernard Zhao wrote:
>> >> The variable fi and bi only used in branch if (!dev->prev_siblings)
>> >> , fi & bi not kmalloc in else branch, so move kfree into branch
>> >> if (!dev->prev_siblings),this change is to cleanup the code a bit.
>> >
>> >Please move the variable declaration into that scope, too. Adjust the
>> >error handling accordingly.
>>
>> Hi Marc:
>>
>> I am not sure if there is some gap.
>> If we move the variable declaration into that scope, then each error branch has to do the kfree job, like:
>> if (err) {
>> dev_err(dev->netdev->dev.parent,
>> "unable to read %s firmware info (err %d)\n",
>> pcan_usb_pro.name, err);
>> kfree(bi);
>> kfree(fi);
>> kfree(usb_if);
>>
>> return err;
>> }
>> I am not sure if this looks a little less clear?
>> Thanks!
>
>A cleaner way would be to move all the content of the if
>(!dev->prev_siblings) to a new function.
Hi Vincent Mailhol:
Got it.
This seems to be a good idea, i would resubmit one patch V2.
Thanks!
BR//Bernard
>
>Yours sincerely,
>Vincent Mailhol
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] usb/peak_usb: cleanup code
2022-05-11 8:28 ` Vincent MAILHOL
2022-05-11 8:46 ` z
@ 2022-05-11 8:47 ` Marc Kleine-Budde
1 sibling, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2022-05-11 8:47 UTC (permalink / raw)
To: Vincent MAILHOL
Cc: z, Wolfgang Grandegger, David S. Miller, Jakub Kicinski,
Paolo Abeni, Stefan Mätje, linux-can, netdev, linux-kernel,
bernard
[-- Attachment #1: Type: text/plain, Size: 1642 bytes --]
On 11.05.2022 17:28:26, Vincent MAILHOL wrote:
> On Wed. 11 May 2022 at 16:11, z <zhaojunkui2008@126.com> wrote:
> > At 2022-05-11 14:44:50, "Marc Kleine-Budde" <mkl@pengutronix.de> wrote:
> > >On 10.05.2022 23:38:38, Bernard Zhao wrote:
> > >> The variable fi and bi only used in branch if (!dev->prev_siblings)
> > >> , fi & bi not kmalloc in else branch, so move kfree into branch
> > >> if (!dev->prev_siblings),this change is to cleanup the code a bit.
> > >
> > >Please move the variable declaration into that scope, too. Adjust the
> > >error handling accordingly.
> >
> > Hi Marc:
> >
> > I am not sure if there is some gap.
> > If we move the variable declaration into that scope, then each error branch has to do the kfree job, like:
> > if (err) {
> > dev_err(dev->netdev->dev.parent,
> > "unable to read %s firmware info (err %d)\n",
> > pcan_usb_pro.name, err);
> > kfree(bi);
> > kfree(fi);
> > kfree(usb_if);
> >
> > return err;
> > }
> > I am not sure if this looks a little less clear?
> > Thanks!
>
> A cleaner way would be to move all the content of the if
> (!dev->prev_siblings) to a new function.
Good idea.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-05-11 8:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-11 6:38 [PATCH] usb/peak_usb: cleanup code Bernard Zhao
2022-05-11 6:44 ` Marc Kleine-Budde
2022-05-11 7:11 ` z
2022-05-11 8:28 ` Vincent MAILHOL
2022-05-11 8:46 ` z
2022-05-11 8:47 ` Marc Kleine-Budde
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).