lkml.org 
[lkml]   [2018]   [Mar]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3] staging: vt6655: check for memory allocation failures
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.

@@ -550,20 +554,29 @@ static void device_init_rd0_ring(struct vnt_private *priv)
+ return 0;
+error:
+ device_free_rd0_ring(priv);
+ return ret;

Would freeing in the vnt_start() be better instead of freeing in
device_init_rd0_ring function?

Best regards,
Ji-Hun

\
 
 \ /
  Last update: 2018-03-30 05:40    [W:0.149 / U:0.112 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site