All of lore.kernel.org
 help / color / mirror / Atom feed
From: "H. Peter Anvin" <hpa@zytor.com>
To: "Randy.Dunlap" <rdunlap@xenotime.net>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>,
	linux-next@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
	x86@kernel.org
Subject: Re: linux-next: Tree for September 16 (cpustr.h)
Date: Tue, 16 Sep 2008 13:48:25 -0700	[thread overview]
Message-ID: <48D01B99.3040908@zytor.com> (raw)
In-Reply-To: <Pine.LNX.4.64.0809161147320.32267@shark.he.net>

[-- Attachment #1: Type: text/plain, Size: 1921 bytes --]

Randy.Dunlap wrote:
> I'm seeing lots of gcc warnings from the generated cpustr.h file, like so:
> 
>   arch/x86/boot/mkcpustr > arch/x86/boot/cpustr.h
>   gcc -Wp,-MD,arch/x86/boot/.cpu.o.d  -nostdinc -isystem /usr/lib/gcc/x86_64-redhat-linux/4.1.1/include -D__KERNEL__ -Iinclude -Iinclude2 -I/home/rdunlap/linsrc/linux-next-20080916/include -I/home/rdunlap/linsrc/linux-next-20080916/arch/x86/include -include include/linux/autoconf.h -I/home/rdunlap/linsrc/linux-next-20080916/arch/x86/boot -Iarch/x86/boot -I/home/rdunlap/linsrc/linux-next-20080916/include -Iinclude -I/home/rdunlap/linsrc/linux-next-20080916/include2 -Iinclude2  -I/home/rdunlap/linsrc/linux-next-20080916/include  -I/home/rdunlap/linsrc/linux-next-20080916/arch/x86/include -include include/linux/autoconf.h -g -Os -D_SETUP -D__KERNEL__ -Wall -Wstrict-prototypes -march=i386 -mregparm=3 -include /home/rdunlap/linsrc/linux-next-20080916/arch/x86/boot/code16gcc.h -fno-strict-alia
 sing -fomit-frame-pointer -ffreestanding -fno-unit-at-a-time -fno-stack-protector -mpreferred-stack-boundary=2 -m32 -D__BIG_KERNEL__  -D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(cpu)"
  -D"KBUILD_MODNAME=KBUILD_STR(cpu)" -D"DEBUG_HASH=3" -D"DEBUG_HASH2=23" -c -o arch/x86/boot/cpu.o /home/rdunlap/linsrc/linux-next-20080916/arch/x86/boot/cpu.c
> cc1: warning: hex escape sequence out of range
> 
> 
> The ending lines of cpustr.h include hex strings like "\x111".
> gcc complains about these.  If I change them to "\x011" (e.g.), there
> is no warning.
> 

Yes, there is a patch that adds a 9th cpuflags word, which this code 
doesn't handle.  The warning is real, there is an #error in mkcpustr.c 
which is supposed to trigger, but doesn't because of a typo:

#if NCAPFLAGS > 8
# error "Need to adjust the boot code handling of CPUID strings"
#endif

(The actual variable is called NCAPINTS.)

Attached is a completely untested patch for this problem.

	-hpa

[-- Attachment #2: diff --]
[-- Type: text/plain, Size: 2230 bytes --]

diff --git a/arch/x86/boot/cpu.c b/arch/x86/boot/cpu.c
index 75298fe..6ec6bb6 100644
--- a/arch/x86/boot/cpu.c
+++ b/arch/x86/boot/cpu.c
@@ -59,17 +59,18 @@ int validate_cpu(void)
 			u32 e = err_flags[i];
 
 			for (j = 0; j < 32; j++) {
-				int n = (i << 5)+j;
-				if (*msg_strs < n) {
+				if (msg_strs[0] < i ||
+				    (msg_strs[0] == i && msg_strs[1] < j)) {
 					/* Skip to the next string */
-					do {
-						msg_strs++;
-					} while (*msg_strs);
-					msg_strs++;
+					msg_strs += 2;
+					while (*msg_strs++)
+						;
 				}
 				if (e & 1) {
-					if (*msg_strs == n && msg_strs[1])
-						printf("%s ", msg_strs+1);
+					if (msg_strs[0] == i &&
+					    msg_strs[1] == j &&
+					    msg_strs[2])
+						printf("%s ", msg_strs+2);
 					else
 						printf("%d:%d ", i, j);
 				}
diff --git a/arch/x86/boot/mkcpustr.c b/arch/x86/boot/mkcpustr.c
index 4589caa..8ef60f2 100644
--- a/arch/x86/boot/mkcpustr.c
+++ b/arch/x86/boot/mkcpustr.c
@@ -17,31 +17,31 @@
 
 #include "../kernel/cpu/capflags.c"
 
-#if NCAPFLAGS > 8
-# error "Need to adjust the boot code handling of CPUID strings"
-#endif
-
 int main(void)
 {
-	int i;
+	int i, j;
 	const char *str;
 
 	printf("static const char x86_cap_strs[] = \n");
 
-	for (i = 0; i < NCAPINTS*32; i++) {
-		str = x86_cap_flags[i];
-
-		if (i == NCAPINTS*32-1) {
-			/* The last entry must be unconditional; this
-			   also consumes the compiler-added null character */
-			if (!str)
-				str = "";
-			printf("\t\"\\x%02x\"\"%s\"\n", i, str);
-		} else if (str) {
-			printf("#if REQUIRED_MASK%d & (1 << %d)\n"
-			       "\t\"\\x%02x\"\"%s\\0\"\n"
-			       "#endif\n",
-			       i >> 5, i & 31, i, str);
+	for (i = 0; i < NCAPINTS; i++) {
+		for (j = 0; j < 32; j++) {
+			str = x86_cap_flags[i*32+j];
+
+			if (i == NCAPINTS-1 && j == 31) {
+				/* The last entry must be unconditional; this
+				   also consumes the compiler-added null
+				   character */
+				if (!str)
+					str = "";
+				printf("\t\"\\x%02x\\x%02x\"\"%s\"\n",
+				       i, j, str);
+			} else if (str) {
+				printf("#if REQUIRED_MASK%d & (1 << %d)\n"
+				       "\t\"\\x%02x\\x%02x\"\"%s\\0\"\n"
+				       "#endif\n",
+				       i, j, i, j, str);
+			}
 		}
 	}
 	printf("\t;\n");

WARNING: multiple messages have this Message-ID (diff)
From: "H. Peter Anvin" <hpa@zytor.com>
To: "Randy.Dunlap" <rdunlap@xenotime.net>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>,
	linux-next@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
	x86@kernel.org
Subject: Re: linux-next: Tree for September 16 (cpustr.h)
Date: Tue, 16 Sep 2008 13:48:25 -0700	[thread overview]
Message-ID: <48D01B99.3040908@zytor.com> (raw)
In-Reply-To: <Pine.LNX.4.64.0809161147320.32267@shark.he.net>

[-- Attachment #1: Type: text/plain, Size: 1919 bytes --]

Randy.Dunlap wrote:
> I'm seeing lots of gcc warnings from the generated cpustr.h file, like so:
> 
>   arch/x86/boot/mkcpustr > arch/x86/boot/cpustr.h
>   gcc -Wp,-MD,arch/x86/boot/.cpu.o.d  -nostdinc -isystem /usr/lib/gcc/x86_64-redhat-linux/4.1.1/include -D__KERNEL__ -Iinclude -Iinclude2 -I/home/rdunlap/linsrc/linux-next-20080916/include -I/home/rdunlap/linsrc/linux-next-20080916/arch/x86/include -include include/linux/autoconf.h -I/home/rdunlap/linsrc/linux-next-20080916/arch/x86/boot -Iarch/x86/boot -I/home/rdunlap/linsrc/linux-next-20080916/include -Iinclude -I/home/rdunlap/linsrc/linux-next-20080916/include2 -Iinclude2  -I/home/rdunlap/linsrc/linux-next-20080916/include  -I/home/rdunlap/linsrc/linux-next-20080916/arch/x86/include -include include/linux/autoconf.h -g -Os -D_SETUP -D__KERNEL__ -Wall -Wstrict-prototypes -march=i386 -mregparm=3 -include /home/rdunlap/linsrc/linux-next-20080916/arch/x86/boot/code16gcc.h -fno-strict-aliasing -fomit-frame-pointer -ffreestanding -fno-unit-at-a-time -fno-stack-protector -mpreferred-stack-boundary=2 -m32 -D__BIG_KERNEL__  -D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(cpu)"
  -D"KBUILD_MODNAME=KBUILD_STR(cpu)" -D"DEBUG_HASH=3" -D"DEBUG_HASH2=23" -c -o arch/x86/boot/cpu.o /home/rdunlap/linsrc/linux-next-20080916/arch/x86/boot/cpu.c
> cc1: warning: hex escape sequence out of range
> 
> 
> The ending lines of cpustr.h include hex strings like "\x111".
> gcc complains about these.  If I change them to "\x011" (e.g.), there
> is no warning.
> 

Yes, there is a patch that adds a 9th cpuflags word, which this code 
doesn't handle.  The warning is real, there is an #error in mkcpustr.c 
which is supposed to trigger, but doesn't because of a typo:

#if NCAPFLAGS > 8
# error "Need to adjust the boot code handling of CPUID strings"
#endif

(The actual variable is called NCAPINTS.)

Attached is a completely untested patch for this problem.

	-hpa

[-- Attachment #2: diff --]
[-- Type: text/plain, Size: 2230 bytes --]

diff --git a/arch/x86/boot/cpu.c b/arch/x86/boot/cpu.c
index 75298fe..6ec6bb6 100644
--- a/arch/x86/boot/cpu.c
+++ b/arch/x86/boot/cpu.c
@@ -59,17 +59,18 @@ int validate_cpu(void)
 			u32 e = err_flags[i];
 
 			for (j = 0; j < 32; j++) {
-				int n = (i << 5)+j;
-				if (*msg_strs < n) {
+				if (msg_strs[0] < i ||
+				    (msg_strs[0] == i && msg_strs[1] < j)) {
 					/* Skip to the next string */
-					do {
-						msg_strs++;
-					} while (*msg_strs);
-					msg_strs++;
+					msg_strs += 2;
+					while (*msg_strs++)
+						;
 				}
 				if (e & 1) {
-					if (*msg_strs == n && msg_strs[1])
-						printf("%s ", msg_strs+1);
+					if (msg_strs[0] == i &&
+					    msg_strs[1] == j &&
+					    msg_strs[2])
+						printf("%s ", msg_strs+2);
 					else
 						printf("%d:%d ", i, j);
 				}
diff --git a/arch/x86/boot/mkcpustr.c b/arch/x86/boot/mkcpustr.c
index 4589caa..8ef60f2 100644
--- a/arch/x86/boot/mkcpustr.c
+++ b/arch/x86/boot/mkcpustr.c
@@ -17,31 +17,31 @@
 
 #include "../kernel/cpu/capflags.c"
 
-#if NCAPFLAGS > 8
-# error "Need to adjust the boot code handling of CPUID strings"
-#endif
-
 int main(void)
 {
-	int i;
+	int i, j;
 	const char *str;
 
 	printf("static const char x86_cap_strs[] = \n");
 
-	for (i = 0; i < NCAPINTS*32; i++) {
-		str = x86_cap_flags[i];
-
-		if (i == NCAPINTS*32-1) {
-			/* The last entry must be unconditional; this
-			   also consumes the compiler-added null character */
-			if (!str)
-				str = "";
-			printf("\t\"\\x%02x\"\"%s\"\n", i, str);
-		} else if (str) {
-			printf("#if REQUIRED_MASK%d & (1 << %d)\n"
-			       "\t\"\\x%02x\"\"%s\\0\"\n"
-			       "#endif\n",
-			       i >> 5, i & 31, i, str);
+	for (i = 0; i < NCAPINTS; i++) {
+		for (j = 0; j < 32; j++) {
+			str = x86_cap_flags[i*32+j];
+
+			if (i == NCAPINTS-1 && j == 31) {
+				/* The last entry must be unconditional; this
+				   also consumes the compiler-added null
+				   character */
+				if (!str)
+					str = "";
+				printf("\t\"\\x%02x\\x%02x\"\"%s\"\n",
+				       i, j, str);
+			} else if (str) {
+				printf("#if REQUIRED_MASK%d & (1 << %d)\n"
+				       "\t\"\\x%02x\\x%02x\"\"%s\\0\"\n"
+				       "#endif\n",
+				       i, j, i, j, str);
+			}
 		}
 	}
 	printf("\t;\n");

  reply	other threads:[~2008-09-16 20:48 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-09-16  7:34 linux-next: Tree for September 16 Stephen Rothwell
2008-09-16 18:00 ` Takashi Iwai
2008-09-16 18:50 ` linux-next: Tree for September 16 (cpustr.h) Randy.Dunlap
2008-09-16 20:48   ` H. Peter Anvin [this message]
2008-09-16 20:48     ` H. Peter Anvin
2008-09-19 20:28     ` Randy Dunlap
2008-09-19 20:28       ` Randy Dunlap
2008-09-16 19:29 ` [PATCH -next] blkdev.h: fix warnings when CONFIG_BLOCK=n Randy.Dunlap
2008-09-16 21:50   ` Jens Axboe
2008-09-16 22:16     ` Stephen Rothwell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=48D01B99.3040908@zytor.com \
    --to=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-next@vger.kernel.org \
    --cc=rdunlap@xenotime.net \
    --cc=sfr@canb.auug.org.au \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.