[patch 1/3] early_printk - use sizeof instead of hardcoded number

[patch 1/3] early_printk - use sizeof instead of hardcoded number

[Cyrill Gorcunov]

[-- Attachment #1: x86-early_printk-buf-sizeof --]
[-- Type: text/plain, Size: 636 bytes --]

Impact: cleanup

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 arch/x86/kernel/early_printk.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.git/arch/x86/kernel/early_printk.c
===================================================================
--- linux-2.6.git.orig/arch/x86/kernel/early_printk.c
+++ linux-2.6.git/arch/x86/kernel/early_printk.c
@@ -886,7 +886,7 @@ asmlinkage void early_printk(const char 
 	va_list ap;
 
 	va_start(ap, fmt);
-	n = vscnprintf(buf, 512, fmt, ap);
+	n = vscnprintf(buf, sizeof(buf), fmt, ap);
 	early_console->write(early_console, buf, n);
 	va_end(ap);
 }

-- 

[patch 2/3] mm: hugetlb -- get rid of redundant if operation

[Cyrill Gorcunov]

[-- Attachment #1: mm-hugetlb --]
[-- Type: text/plain, Size: 684 bytes --]

At this point we already know that 'addr' is not NULL
so get rid of redundant 'if'. Probably gcc eliminate it
by optimization pass.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
CC: William Irwin <wli@holomorphy.com>
---
 mm/hugetlb.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: linux-2.6.git/mm/hugetlb.c
===================================================================
--- linux-2.6.git.orig/mm/hugetlb.c
+++ linux-2.6.git/mm/hugetlb.c
@@ -991,8 +991,7 @@ __attribute__((weak)) int alloc_bootmem_
 			 * puts them into the mem_map).
 			 */
 			m = addr;
-			if (m)
-				goto found;
+			goto found;
 		}
 		hstate_next_node(h);
 		nr_nodes--;

-- 

[patch 3/3] acpi: check for pxm_to_node_map overflow

[Cyrill Gorcunov]

[-- Attachment #1: x86-acpi-pxm-overflow --]
[-- Type: text/plain, Size: 786 bytes --]

It is hardly (if ever) possible but in case of broken _PXM
entry we could reach out of pxm_to_node_map array
bounds in acpi_map_pxm_to_node() call. Lets check it
(it's not that expensive and safe).

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
CC: Len Brown <lenb@kernel.org>
---
 drivers/acpi/numa.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.git/drivers/acpi/numa.c
===================================================================
--- linux-2.6.git.orig/drivers/acpi/numa.c
+++ linux-2.6.git/drivers/acpi/numa.c
@@ -278,7 +278,7 @@ int acpi_get_node(acpi_handle *handle)
 	int pxm, node = -1;
 
 	pxm = acpi_get_pxm(handle);
-	if (pxm >= 0)
+	if (pxm >= 0 && pxm < MAX_PXM_DOMAINS)
 		node = acpi_map_pxm_to_node(pxm);
 
 	return node;

-- 

Re: [patch 2/3] mm: hugetlb -- get rid of redundant if operation

[Ingo Molnar]

Andrew Cc:-ed.

patch looks correct to me.

Reviewed-by: Ingo Molnar <mingo@elte.hu>

	Ingo

* Cyrill Gorcunov <gorcunov@gmail.com> wrote:

> At this point we already know that 'addr' is not NULL
> so get rid of redundant 'if'. Probably gcc eliminate it
> by optimization pass.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> CC: William Irwin <wli@holomorphy.com>
> ---
>  mm/hugetlb.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> Index: linux-2.6.git/mm/hugetlb.c
> ===================================================================
> --- linux-2.6.git.orig/mm/hugetlb.c
> +++ linux-2.6.git/mm/hugetlb.c
> @@ -991,8 +991,7 @@ __attribute__((weak)) int alloc_bootmem_
>  			 * puts them into the mem_map).
>  			 */
>  			m = addr;
> -			if (m)
> -				goto found;
> +			goto found;
>  		}
>  		hstate_next_node(h);
>  		nr_nodes--;
> 
> -- 

Re: [patch 3/3] acpi: check for pxm_to_node_map overflow

[Ingo Molnar]

* Cyrill Gorcunov <gorcunov@gmail.com> wrote:

> It is hardly (if ever) possible but in case of broken _PXM
> entry we could reach out of pxm_to_node_map array
> bounds in acpi_map_pxm_to_node() call. Lets check it
> (it's not that expensive and safe).
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> CC: Len Brown <lenb@kernel.org>
> ---
>  drivers/acpi/numa.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

just curious - how did you find this - code review or some tool help like 
Sparse output or a compiler warning?

	Ingo

Re: [patch 3/3] acpi: check for pxm_to_node_map overflow

[Cyrill Gorcunov]

[Ingo Molnar - Fri, Jan 02, 2009 at 10:23:30AM +0100]
| 
| * Cyrill Gorcunov <gorcunov@gmail.com> wrote:
| 
| > It is hardly (if ever) possible but in case of broken _PXM
| > entry we could reach out of pxm_to_node_map array
| > bounds in acpi_map_pxm_to_node() call. Lets check it
| > (it's not that expensive and safe).
| > 
| > Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
| > CC: Len Brown <lenb@kernel.org>
| > ---
| >  drivers/acpi/numa.c |    2 +-
| >  1 file changed, 1 insertion(+), 1 deletion(-)
| 
| just curious - how did you find this - code review or some tool help like 
| Sparse output or a compiler warning?
| 
| 	Ingo
| 

Unfortunately -- by reading the code :( I would prefer
if gcc or sparse complain about it but we have indirect
referring which is that hard to trace by any tool I believe
(until say some emulator but even if we have such kind of
tool we would need to tell it that 'there we have boundries
we shouldn't cross)'. But to be fair -- I didn't try sparse
here maybe it does complain :)

		- Cyrill -

Re: [patch 3/3] acpi: check for pxm_to_node_map overflow

[Cyrill Gorcunov]

[Cyrill Gorcunov - Fri, Jan 02, 2009 at 11:27:20AM +0300]
| It is hardly (if ever) possible but in case of broken _PXM
| entry we could reach out of pxm_to_node_map array
| bounds in acpi_map_pxm_to_node() call. Lets check it
| (it's not that expensive and safe).
| 
| Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
| CC: Len Brown <lenb@kernel.org>
...

Oops, I forgot to CC Andi, sorry.

		- Cyrill -