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 > > Oh, sorry, I got what you said. Yes, you are right. I am going to make patch v4. Thanks! Best regards, Ji-Hun -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html