linux-kernel - Re: [PATCH] kernel/smp.c: free related resources when failure occurs in hotplug

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130708142605.GA2786@windriver.com>
Date:	Mon, 8 Jul 2013 10:26:05 -0400
From:	Paul Gortmaker <paul.gortmaker@...driver.com>
To:	Chen Gang <gang.chen@...anux.com>
CC:	<lig.fnst@...fujitsu.com>, <chuansheng.liu@...el.com>,
	Wang YanQing <udknight@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] kernel/smp.c: free related resources when failure occurs
 in hotplug_cfd()

[[PATCH] kernel/smp.c: free related resources when failure occurs in hotplug_cfd()] On 08/07/2013 (Mon 16:50) Chen Gang wrote:

> When failure occurs in hotplug_cfd(), need release related resources,
> or will cause memory leak.
> 
> Also beautify the related code.

No.  Please do not mix real fixes with trivial whitespace changes.
It makes it harder for the reviewer to find the actual fix, and it
makes the fix less portable to other releases (i.e. stable trees.)

Also, you say "beautify", but that is a matter of opinion.  You
shuffle around the tabs in your whitespace change, and yet even
then you don't manage to adapt it to the general coding style of
having multi-line args align with where the 1st arg starts.  So
you have done nothing but pollute the "git blame" history of that
file for other users.

You might want to slow down on the quantity of patches you send,
and spend more time reading the comments from other people on
reviewed patches and learning some of the implicit requirements
from those.  I've noticed that you are already dangerously close
to annoying several key subsystem maintainers, and that is not
the right long term approach to working with the linux community.

Paul.
--

> 
> Signed-off-by: Chen Gang <gang.chen@...anux.com>
> ---
>  kernel/smp.c |   13 +++++++++----
>  1 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/smp.c b/kernel/smp.c
> index 02a885d..c264623 100644
> --- a/kernel/smp.c
> +++ b/kernel/smp.c
> @@ -45,15 +45,20 @@ hotplug_cfd(struct notifier_block *nfb, unsigned long action, void *hcpu)
>  	switch (action) {
>  	case CPU_UP_PREPARE:
>  	case CPU_UP_PREPARE_FROZEN:
> -		if (!zalloc_cpumask_var_node(&cfd->cpumask, GFP_KERNEL,
> -				cpu_to_node(cpu)))
> +		if (!zalloc_cpumask_var_node(&cfd->cpumask,
> +						GFP_KERNEL, cpu_to_node(cpu)))
>  			return notifier_from_errno(-ENOMEM);
> -		if (!zalloc_cpumask_var_node(&cfd->cpumask_ipi, GFP_KERNEL,
> -				cpu_to_node(cpu)))
> +
> +		if (!zalloc_cpumask_var_node(&cfd->cpumask_ipi,
> +						GFP_KERNEL, cpu_to_node(cpu))) {
> +			free_cpumask_var(cfd->cpumask);
>  			return notifier_from_errno(-ENOMEM);
> +		}
> +
>  		cfd->csd = alloc_percpu(struct call_single_data);
>  		if (!cfd->csd) {
>  			free_cpumask_var(cfd->cpumask);
> +			free_cpumask_var(cfd->cpumask_ipi);
>  			return notifier_from_errno(-ENOMEM);
>  		}
>  		break;
> -- 
> 1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/