On 11.05.2022 17:28:26, Vincent MAILHOL wrote: > On Wed. 11 May 2022 at 16:11, z <zhaojunkui2008@xxxxxxx> wrote: > > At 2022-05-11 14:44:50, "Marc Kleine-Budde" <mkl@xxxxxxxxxxxxxx> 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:
signature.asc
Description: PGP signature