Re: [PATCH] wl18xx: Handle kfree() in better way when kzalloc fails

2016-10-07 Thread souptick joarder
On Fri, Oct 7, 2016 at 12:36 PM, Johannes Berg
 wrote:
>
> On Fri, 2016-10-07 at 12:19 +0530, Souptick Joarder wrote:
> > This patch is added to handle kfree and return error in a better way
>
> What's "better" about this?
>
> kfree(NULL) is perfectly valid, adding another label makes the code
> harder to read,

I agree with you.

when kzalloc(sizeof(*cmd), GFP_KERNEL) fails, try to avoid extra
kfree(cmd_channel) function call,
cause anyway it will call kfree with NULL.

I thought from that point of view.

>
> > - struct wl18xx_cmd_scan_params *cmd;
> > + struct wl18xx_cmd_scan_params *cmd = NULL;
>
> that new initialization is actually *completely* pointless since it's
> overwritten immediately here:
>
> >   struct wlcore_scan_channels *cmd_channels = NULL;
> >   int ret;
> >
> >   cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
> >   if (!cmd) {
> > - ret = -ENOMEM;
> > - goto out;
> > + return -ENOMEM;
> >   }
> >
> ...
>
> what gives?
>

> johannes

Ok, I will drop this patch.

-Souptick


Re: [PATCH] wl18xx: Handle kfree() in better way when kzalloc fails

2016-10-07 Thread Johannes Berg
On Fri, 2016-10-07 at 12:19 +0530, Souptick Joarder wrote:
> This patch is added to handle kfree and return error in a better way

What's "better" about this?

kfree(NULL) is perfectly valid, adding another label makes the code
harder to read,

> - struct wl18xx_cmd_scan_params *cmd;
> + struct wl18xx_cmd_scan_params *cmd = NULL;

that new initialization is actually *completely* pointless since it's
overwritten immediately here:

>   struct wlcore_scan_channels *cmd_channels = NULL;
>   int ret;
>  
>   cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
>   if (!cmd) {
> - ret = -ENOMEM;
> - goto out;
> + return -ENOMEM;
>   }
> 
...

what gives?

johannes