2017-08-23 14:29:43

by Himanshu Jha

Subject: [PATCH v2] at76c50x-usb: check memory allocation failure

Check memory allocation failure and return -ENOMEM rather than just
retuning void.

Signed-off-by: Himanshu Jha <[email protected]>
---
drivers/net/wireless/atmel/at76c50x-usb.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/atmel/at76c50x-usb.c b/drivers/net/wireless/atmel/at76c50x-usb.c
index 09defbc..ceb453b 100644
--- a/drivers/net/wireless/atmel/at76c50x-usb.c
+++ b/drivers/net/wireless/atmel/at76c50x-usb.c
@@ -932,7 +932,7 @@ static int at76_set_autorate_fallback(struct at76_priv *priv, int onoff)
return ret;
}

-static void at76_dump_mib_mac_addr(struct at76_priv *priv)
+static int at76_dump_mib_mac_addr(struct at76_priv *priv)
{
int i;
int ret;
@@ -940,7 +940,7 @@ static void at76_dump_mib_mac_addr(struct at76_priv *priv)
GFP_KERNEL);

if (!m)
- return;
+ return -ENOMEM;

ret = at76_get_mib(priv->udev, MIB_MAC_ADDR, m,
sizeof(struct mib_mac_addr));
@@ -961,7 +961,7 @@ static void at76_dump_mib_mac_addr(struct at76_priv *priv)
kfree(m);
}

-static void at76_dump_mib_mac_wep(struct at76_priv *priv)
+static int at76_dump_mib_mac_wep(struct at76_priv *priv)
{
int i;
int ret;
@@ -969,7 +969,7 @@ static void at76_dump_mib_mac_wep(struct at76_priv *priv)
struct mib_mac_wep *m = kmalloc(sizeof(struct mib_mac_wep), GFP_KERNEL);

if (!m)
- return;
+ return -ENOMEM;

ret = at76_get_mib(priv->udev, MIB_MAC_WEP, m,
sizeof(struct mib_mac_wep));
@@ -999,14 +999,14 @@ static void at76_dump_mib_mac_wep(struct at76_priv *priv)
kfree(m);
}

-static void at76_dump_mib_mac_mgmt(struct at76_priv *priv)
+static int at76_dump_mib_mac_mgmt(struct at76_priv *priv)
{
int ret;
struct mib_mac_mgmt *m = kmalloc(sizeof(struct mib_mac_mgmt),
GFP_KERNEL);

if (!m)
- return;
+ return -ENOMEM;

ret = at76_get_mib(priv->udev, MIB_MAC_MGMT, m,
sizeof(struct mib_mac_mgmt));
@@ -1037,13 +1037,13 @@ static void at76_dump_mib_mac_mgmt(struct at76_priv *priv)
kfree(m);
}

-static void at76_dump_mib_mac(struct at76_priv *priv)
+static int at76_dump_mib_mac(struct at76_priv *priv)
{
int ret;
struct mib_mac *m = kmalloc(sizeof(struct mib_mac), GFP_KERNEL);

if (!m)
- return;
+ return -ENOMEM;

ret = at76_get_mib(priv->udev, MIB_MAC, m, sizeof(struct mib_mac));
if (ret < 0) {
@@ -1074,13 +1074,13 @@ static void at76_dump_mib_mac(struct at76_priv *priv)
kfree(m);
}

-static void at76_dump_mib_phy(struct at76_priv *priv)
+static int at76_dump_mib_phy(struct at76_priv *priv)
{
int ret;
struct mib_phy *m = kmalloc(sizeof(struct mib_phy), GFP_KERNEL);

if (!m)
- return;
+ return -ENOMEM;

ret = at76_get_mib(priv->udev, MIB_PHY, m, sizeof(struct mib_phy));
if (ret < 0) {
@@ -1107,13 +1107,13 @@ static void at76_dump_mib_phy(struct at76_priv *priv)
kfree(m);
}

-static void at76_dump_mib_local(struct at76_priv *priv)
+static int at76_dump_mib_local(struct at76_priv *priv)
{
int ret;
struct mib_local *m = kmalloc(sizeof(*m), GFP_KERNEL);

if (!m)
- return;
+ return -ENOMEM;

ret = at76_get_mib(priv->udev, MIB_LOCAL, m, sizeof(*m));
if (ret < 0) {
@@ -1132,13 +1132,13 @@ static void at76_dump_mib_local(struct at76_priv *priv)
kfree(m);
}

-static void at76_dump_mib_mdomain(struct at76_priv *priv)
+static int at76_dump_mib_mdomain(struct at76_priv *priv)
{
int ret;
struct mib_mdomain *m = kmalloc(sizeof(struct mib_mdomain), GFP_KERNEL);

if (!m)
- return;
+ return -ENOMEM;

ret = at76_get_mib(priv->udev, MIB_MDOMAIN, m,
sizeof(struct mib_mdomain));
--
2.7.4

2017-08-23 14:49:20

by Kalle Valo

Subject: Re: [PATCH v2] at76c50x-usb: check memory allocation failure

Himanshu Jha <[email protected]> writes:

> Check memory allocation failure and return -ENOMEM rather than just
> retuning void.
>
> Signed-off-by: Himanshu Jha <[email protected]>

[...]

> -static void at76_dump_mib_mac_addr(struct at76_priv *priv)
> +static int at76_dump_mib_mac_addr(struct at76_priv *priv)
> {
> int i;
> int ret;
> @@ -940,7 +940,7 @@ static void at76_dump_mib_mac_addr(struct at76_priv *priv)
> GFP_KERNEL);
>
> if (!m)
> - return;
> + return -ENOMEM;

This feels rather pointless. You are changing these functions to return
an error code but the callers it anyway. And besides, did you check
where these are actually used?

if (at76_debug & DBG_MIB) {
at76_dump_mib_mac(priv);
at76_dump_mib_mac_addr(priv);
at76_dump_mib_mac_mgmt(priv);
at76_dump_mib_mac_wep(priv);
at76_dump_mib_mdomain(priv);
at76_dump_mib_phy(priv);
at76_dump_mib_local(priv);
}

So it's just for debug messages and the current error handling looks the
correct approach. There is not much anything else we can do than just
skip the message printout.

--
Kalle Valo