Re: [PATCH 3/5] mt76: fix a leaked reference by adding a missingof_node_put

[<wen.yang99-Th6q7B73Y6EnDS1+zs4M5A@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1307 bytes --]

> > @@ -54,22 +54,30 @@  mt76_get_of_eeprom(struct mt76_dev *dev, int len)
> >          part = np->name;
> >
> >      mtd = get_mtd_device_nm(part);
> …
> > +    if (retlen < len) {
> > +        ret = -EINVAL;
> > +        goto out_put_node;
> 
> I find a jump to an immediately following source code place unnecessary.
> Would you like to delete it?
> 
> 
> > +    }
> >
> > -    return 0;
> > +out_put_node:
> > +    of_node_put(np);
> > +    return ret;
> >  #else
> 
> Can another bit of fine-tuning matter here?
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/net/wireless/mediatek/mt76/eeprom.c?id=34e022d8b780a03902d82fb3997ba7c7b1f40c81#n73
> 

Hi Markus,
Thank you for your comments.
We may have some different opinions here.
Deleting the goto statement may not be good.
If the code further up is changed it's easy enough to miss that a goto statement needs to be added here.

Better to set ret to zero explicitly, this is the success path after all.

--- a/drivers/net/wireless/mediatek/mt76/eeprom.c
+++ b/drivers/net/wireless/mediatek/mt76/eeprom.c
@@ -75,6 +75,8 @@ mt76_get_of_eeprom(struct mt76_dev *dev, int len)
                goto out_put_node;
        }

+       ret = 0;
+
 out_put_node:
        of_node_put(np);
        return ret;

--
Regards,
Wen

[-- Attachment #2: Type: text/plain, Size: 200 bytes --]

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek