staging: r8188eu: handle rtw_init_netdev_name() failure appropriately

PATCH Superseded Vihas Mak 3 participants 5 messages Jan 16, 2022 – Jan 20, 2022 Staging

Share & tools

Vihas Mak

author

2022-01-16 19:26 UTC #0 lore raw

PATCH 1/1 staging: r8188eu: handle rtw_init_netdev_name() failure appropriately

rtw_init_netdev_name() calls dev_alloc_name() which allocates the name for the device as per the given name format.
It returns a negative err code if the format is invalid. Currently the name format is specified by the module parameter "ifname". Warn the user if "ifname" is invalid.

Signed-off-by Vihas Mak <[email protected]>

---
 drivers/staging/r8188eu/os_dep/usb_intf.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

drivers/staging/r8188eu/os_dep/usb_intf.c +4 -1

index 91792dfd3..875815b5e 100644
@@ -399,7 +399,10 @@ static struct adapter *rtw_usb_if1_init(struct dvobj_priv *dvobj,
 			DBG_88E("can't get autopm:\n");
 
 	/*  alloc dev name after read efuse. */
-	rtw_init_netdev_name(pnetdev, padapter->registrypriv.ifname);
+	if (rtw_init_netdev_name(pnetdev, padapter->registrypriv.ifname) < 0) {
+		DBG_88E("rtw_init_netdev_name failed, ifname:%s\n",
+			padapter->registrypriv.ifname);
+	}
 	rtw_macaddr_cfg(padapter->eeprompriv.mac_addr);
 	rtw_init_wifidirect_addrs(padapter, padapter->eeprompriv.mac_addr,
 				  padapter->eeprompriv.mac_addr);

Signature · 1 line

2.30.2

Pavel Skripkin

2022-01-16 21:47 UTC #1 lore raw

Hi Vihas,

Quoted 17 lines · Vihas Mak

rtw_init_netdev_name() calls dev_alloc_name() which allocates the name for the device as per the given name format.
It returns a negative err code if the format is invalid. Currently the name format is specified by the module parameter "ifname". Warn the user if "ifname" is invalid.
Signed-off-by Vihas Mak <[email protected]>
---
  drivers/staging/r8188eu/os_dep/usb_intf.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)
drivers/staging/r8188eu/os_dep/usb_intf.c +0 -0
index 91792dfd3..875815b5e 100644
@@ -399,7 +399,10 @@ static struct adapter *rtw_usb_if1_init(struct dvobj_priv *dvobj,
  			DBG_88E("can't get autopm:\n");

drivers/staging/r8188eu/os_dep/usb_intf.c static struct adapter *rtw_usb_if1_init(struct dvobj_priv *dvobj, · line 408

Quoted 10 lines

drivers/staging/r8188eu/os_dep/usb_intf.c +4 -1

 
  	/*  alloc dev name after read efuse. */
-	rtw_init_netdev_name(pnetdev, padapter->registrypriv.ifname);
+	if (rtw_init_netdev_name(pnetdev, padapter->registrypriv.ifname) < 0) {
+		DBG_88E("rtw_init_netdev_name failed, ifname:%s\n",
+			padapter->registrypriv.ifname);
+	}
  	rtw_macaddr_cfg(padapter->eeprompriv.mac_addr);
  	rtw_init_wifidirect_addrs(padapter, padapter->eeprompriv.mac_addr,
  				  padapter->eeprompriv.mac_addr);

rtw_init_netdev_name() has an error message in case of allocation failure. It looks like it's better to handle the error normally and pass it to caller

With regards,
Pavel Skripkin

Vihas Mak

author

2022-01-17 04:05 UTC #2 lore raw

Quoted 3 lines

Quoted 3 lines
rtw_init_netdev_name() has an error message in case of allocation failure. It looks like it's better to handle the error normally and pass it to caller

By that you mean:

    goto handle_dualmac;

and freeing the adapter.
right?
FYI, if the allocation fails then it assigns the name using default format i.e. "eth%d".

Quoted 41 lines · Pavel Skripkin

Hi Vihas,
Quoted 17 lines · Vihas Mak
rtw_init_netdev_name() calls dev_alloc_name() which allocates the name for the device as per the given name format.
It returns a negative err code if the format is invalid. Currently the name format is specified by the module parameter "ifname". Warn the user if "ifname" is invalid.
Signed-off-by Vihas Mak <[email protected]>
---
  drivers/staging/r8188eu/os_dep/usb_intf.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)
drivers/staging/r8188eu/os_dep/usb_intf.c +0 -0
index 91792dfd3..875815b5e 100644
@@ -399,7 +399,10 @@ static struct adapter *rtw_usb_if1_init(struct dvobj_priv *dvobj,
                      DBG_88E("can't get autopm:\n");
drivers/staging/r8188eu/os_dep/usb_intf.c static struct adapter *rtw_usb_if1_init(struct dvobj_priv *dvobj, · line 408
Quoted 10 lines
drivers/staging/r8188eu/os_dep/usb_intf.c +4 -1
 
      /*  alloc dev name after read efuse. */
-     rtw_init_netdev_name(pnetdev, padapter->registrypriv.ifname);
+     if (rtw_init_netdev_name(pnetdev, padapter->registrypriv.ifname) < 0) {
+             DBG_88E("rtw_init_netdev_name failed, ifname:%s\n",
+                     padapter->registrypriv.ifname);
+     }
      rtw_macaddr_cfg(padapter->eeprompriv.mac_addr);
      rtw_init_wifidirect_addrs(padapter, padapter->eeprompriv.mac_addr,
                                padapter->eeprompriv.mac_addr);
rtw_init_netdev_name() has an error message in case of allocation failure. It looks like it's better to handle the error normally and pass it to caller
With regards,
Pavel Skripkin

Signature · 2 lines

Thanks,
Vihas

Dan Carpenter

2022-01-18 15:08 UTC #3 lore raw

In reply to Vihas Mak

Quoted 8 lines · Vihas Mak

rtw_init_netdev_name() calls dev_alloc_name() which allocates the name for the device as per the given name format.
It returns a negative err code if the format is invalid. Currently the name format is specified by the module parameter "ifname". Warn the user if "ifname" is invalid.
Signed-off-by Vihas Mak <[email protected]>
---

This doesn't really fix the bug. Better to re-write the error handling. See my error handling guide here:

https://lore.kernel.org/all/20210831084735.GL12231@kadam/

regards,
dan carpenter

Vihas Mak

author

2022-01-20 12:29 UTC #4 lore raw

Quoted 2 lines

Quoted 2 lines
This doesn't really fix the bug. Better to re-write the error handling. See my error handling guide here:

Thanks Dan for the guide. Will submit a v2 soon.

Quoted 19 lines · Dan Carpenter

Quoted 8 lines · Vihas Mak
rtw_init_netdev_name() calls dev_alloc_name() which allocates the name for the device as per the given name format.
It returns a negative err code if the format is invalid. Currently the name format is specified by the module parameter "ifname". Warn the user if "ifname" is invalid.
Signed-off-by Vihas Mak <[email protected]>
---
This doesn't really fix the bug. Better to re-write the error handling. See my error handling guide here:
https://lore.kernel.org/all/20210831084735.GL12231@kadam/
regards,
dan carpenter

Signature · 2 lines

Thanks,
Vihas