On 2018/3/30 11:39, Ji-Hun Kim wrote:
On Fri, Mar 30, 2018 at 11:15:03AM +0800, Jia-Ju Bai wrote:
On 2018/3/30 10:44, Ji-Hun Kim wrote:
@@ -1165,10 +1205,18 @@ static int vnt_start(struct ieee80211_hw *hw)
}
dev_dbg(&priv->pcid->dev, "call device init rd0 ring\n");
- device_init_rd0_ring(priv);
- device_init_rd1_ring(priv);
- device_init_td0_ring(priv);
- device_init_td1_ring(priv);
+ ret = device_init_rd0_ring(priv);
+ if (ret)
+ goto error;
+ ret = device_init_rd1_ring(priv);
+ if (ret)
+ goto error;
+ ret = device_init_td0_ring(priv);
+ if (ret)
+ goto error;
+ ret = device_init_td1_ring(priv);
+ if (ret)
+ goto error;
device_init_registers(priv);
@@ -1178,6 +1226,8 @@ static int vnt_start(struct ieee80211_hw *hw)
ieee80211_wake_queues(hw);
return 0;
+error:
+ return ret;
}
This code will lead to memory leaks when device_init_rd1_ring()
fails, because the memory allocated by device_init_rd0_ring() is not
freed.
I think this one will be better:
ret = device_init_rd0_ring(priv);
if (ret)
goto error_init_rd0_ring;
ret = device_init_rd1_ring(priv);
if (ret)
goto error_init_rd1_ring;
ret = device_init_td0_ring(priv);
if (ret)
goto error_init_td0_ring;
ret = device_init_td1_ring(priv);
if (ret)
goto error_init_td1_ring;
......
error_init_td1_ring:
device_free_td0_ring(priv);
error_init_td0_ring:
device_free_rd1_ring(priv);
error_init_rd1_ring:
device_free_rd0_ring(priv);
error_init_rd0_ring:
return ret;
Best wishes,
Jia-Ju Bai
But, those freeing function are already placed in the each device_init
functions for allocation fail like below.
I think it is okay.
I suppose that, for each device_init function, you only free the
resources allocated in this function, and do not handle the resources in
other functions.
@@ -550,20 +554,29 @@ static void device_init_rd0_ring(struct vnt_private *priv)
+ return 0;
+error:
+ device_free_rd0_ring(priv);
+ return ret;
This code is needed to free the resources allocated in this function.
Best wishes,
Jia-Ju Bai
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
