Re: libnuma on big-endian 64-bit systems

[Arnd Bergmann <arnd@arndb.de>]

On Tuesday 09 December 2008, you wrote:
> 
> You had called my attention to problems with libnuma's read_mask() on
> a big-endian 64-bit machine.
> I rolled my proposed patches into numactl-2.0.3-rc1.tar.gz on
> ftp://oss.sgi.com/www/projects/libnuma/download/
> 
> I tested on one 64-bit big-endian system (mips).
> Could you see if that version works on your systems?

Sorry for the long delay. I have now been able to work on this a bit
and come up with a not so broken version of the read_mask function.
The patch you have in 2.0.3-rc1 did not fix the original problem,
but also introduced other bugs, so please revert it.

This is the best I could come up with, haven't tested on little-endian
though because I lack appropriate hardware.

---
From 6009179978adcf787226cfb09e9b8471ded4c8b4 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 20 Jan 2009 20:50:39 +0100
Subject: [PATCH] Next attempt to fix libnuma for bit-endian 64-bit systems

This fixes the read_mask function for me on ppc64. The changes are:

* Correct bit-order on 64 bit systems.
* Fix off-by-one error in counting single-value fields
* No longer skip zero values in the middle of a sparse bitmask.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 libnuma.c |   42 +++++++++++++++++++++++-------------------
 1 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/libnuma.c b/libnuma.c
index 87176ac..f645528 100755
--- a/libnuma.c
+++ b/libnuma.c
@@ -368,11 +368,10 @@ static int
 read_mask(char *s, struct bitmask *bmp)
 {
 	char *end = s;
-	unsigned int *start = (unsigned int *)bmp->maskp;
-	unsigned int *p = start;
-	unsigned int *q;
-	unsigned int i;
-	unsigned int n = 0;
+	int tmplen = (bmp->size + bitsperint - 1) / bitsperint;
+	unsigned int tmp[tmplen];
+	unsigned int *start = tmp;
+	unsigned int i, n = 0, m = 0;
 
 	i = strtoul(s, &end, 16);
 
@@ -380,7 +379,6 @@ read_mask(char *s, struct bitmask *bmp)
 	while (!i && *end++ == ',') {
 		i = strtoul(end, &end, 16);
 	}
-	end++; /* past the , */
 
 	if (!i)
 		/* End of string. No mask */
@@ -388,33 +386,39 @@ read_mask(char *s, struct bitmask *bmp)
 
 	start[n++] = i;
 	/* Read sequence of ints */
-	do {
+	while (*end++ == ',') {
 		i = strtoul(end, &end, 16);
-		if (i)
-			start[n++] = i;
-	} while (*end++ == ',');
-	n--;
+		start[n++] = i;
+
+		/* buffer overflow */
+		if (n > tmplen)
+			return -1;
+	}
 
 	/*
 	 * Invert sequence of ints if necessary since the first int
 	 * is the highest and we put it first because we read it first.
 	 */
-	for (q = start + n, p = start; p < q; q--, p++) {
-		unsigned int x = *q;
-
-		*q = *p;
-		*p = x;
+	while (n) {
+		int w;
+		unsigned long x = 0;
+		/* read into long values in an endian-safe way */
+		for (w = 0; n && w < bitsperlong; w += bitsperint)
+			x |= ((unsigned long)start[n-- - 1] << w);
+
+		bmp->maskp[m++] = x;	
 	}
+	m--;	
 
 	/* Poor mans fls() */
-	for(i = 31; i >= 0; i--)
-		if (test_bit(i, start + n))
+	for(i = bitsperlong - 1; i >= 0; i--)
+		if (test_bit(i, bmp->maskp + m))
 			break;
 
 	/*
 	 * Return the last bit set
 	 */
-	return ((sizeof(unsigned int)*8) * n) + i;
+	return bitsperlong * m + i;
 }
 
 /*
-- 
1.5.4.5