On Mon, Mar 16, 2020 at 1:59 AM Marek Vasut <marek.va...@gmail.com> wrote:
>
> Fix memleak in the init fail path, where if allocation or registration
> of MDIO bus fails, then ethernet interface is not unregistered and the
> private data are not freed, yet the probe function reports a failure.
>
> Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com>
> Cc: Joe Hershberger <joe.hershber...@ni.com>
> Cc: Masahiro Yamada <yamada.masah...@socionext.com>
> ---
> drivers/net/smc911x.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
> index 81f8f0d017..44cb45af61 100644
> --- a/drivers/net/smc911x.c
> +++ b/drivers/net/smc911x.c
> @@ -249,7 +249,7 @@ int smc911x_initialize(u8 dev_num, int base_addr)
>
> dev = calloc(1, sizeof(*dev));
> if (!dev) {
> - return -1;
> + return -ENODEV;
> }
Our convention is to return -ENOMEM
when memory allocation fails.
If you like to do some additional cleanups here,
you can remove the unneeded braces around the single
statement, which will fix the checkpatch warning.
>
> dev->iobase = base_addr;
> @@ -283,15 +283,23 @@ int smc911x_initialize(u8 dev_num, int base_addr)
> #if defined(CONFIG_MII) || defined(CONFIG_CMD_MII)
> int retval;
> struct mii_dev *mdiodev = mdio_alloc();
> - if (!mdiodev)
> + if (!mdiodev) {
> + eth_unregister(dev);
> + free(dev);
> return -ENOMEM;
> + }
> +
> strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
> mdiodev->read = smc911x_miiphy_read;
> mdiodev->write = smc911x_miiphy_write;
>
> retval = mdio_register(mdiodev);
> - if (retval < 0)
> + if (retval < 0) {
> + mdio_free(mdiodev);
> + eth_unregister(dev);
> + free(dev);
> return retval;
Using "goto <label>" is a general tip to
simplify the failure path.
> + }
> #endif
>
> return 1;
> --
> 2.25.0
>
--
Best Regards
Masahiro Yamada
