[PATCH] two missing bits (was Re: Add -Wcast-qual flag)

[PATCH] two missing bits (was Re: Add -Wcast-qual flag)

[Christoph Egger]

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


Hi!

I catched two pieces with debug and crash_debug enabled.
One of them fixes a build error even without the -Wcast-qual flag.

Patch attached.

[-- Attachment #2: xen-cast-xen.diff --]
[-- Type: text/x-diff, Size: 871 bytes --]

diff -r f1ddc050d848 xen/common/gdbstub.c
--- a/xen/common/gdbstub.c	Thu Dec 21 20:44:35 2006 +0000
+++ b/xen/common/gdbstub.c	Fri Dec 22 09:51:42 2006 +0100
@@ -382,7 +382,7 @@ static int
 static int 
 process_command(struct cpu_user_regs *regs, struct gdb_context *ctx)
 {
-    char *ptr;
+    const char *ptr;
     unsigned long addr, length;
     int resume = 0;
 
diff -r f1ddc050d848 xen/include/asm-x86/bitops.h
--- a/xen/include/asm-x86/bitops.h	Thu Dec 21 20:44:35 2006 +0000
+++ b/xen/include/asm-x86/bitops.h	Fri Dec 22 09:48:48 2006 +0100
@@ -246,7 +246,7 @@ static __inline__ int constant_test_bit(
 	return ((1U << (nr & 31)) & (((const volatile unsigned int *) addr)[nr >> 5])) != 0;
 }
 
-static __inline__ int variable_test_bit(int nr, const volatile void * addr)
+static __inline__ int variable_test_bit(int nr, volatile void * addr)
 {
 	int oldbit;
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH] two missing bits (was Re: Add -Wcast-qual flag)

[Jan Beulich]

I object to the change to variable_test_bit() - why is that needed? I had specifically
submitted a patch to get the const qualifier added there. Jan

>>> "Christoph Egger" <Christoph.Egger@amd.com> 22.12.06 10:05 >>>

Hi!

I catched two pieces with debug and crash_debug enabled.
One of them fixes a build error even without the -Wcast-qual flag.

Patch attached.

Re: [PATCH] two missing bits (was Re: Add -Wcast-qual flag)

[Keir Fraser]

Maybe we need a CONST_ADDR macro, as I suspect the problem is the cast
hidden inside the ADDR macro is dropping the const qualifier.

 -- Keir

On 22/12/06 9:31 am, "Jan Beulich" <jbeulich@novell.com> wrote:

> I object to the change to variable_test_bit() - why is that needed? I had
> specifically
> submitted a patch to get the const qualifier added there. Jan
> 
>>>> "Christoph Egger" <Christoph.Egger@amd.com> 22.12.06 10:05 >>>
> 
> Hi!
> 
> I catched two pieces with debug and crash_debug enabled.
> One of them fixes a build error even without the -Wcast-qual flag.
> 
> Patch attached.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] two missing bits (was Re: Add -Wcast-qual flag)

[Christoph Egger]

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

On Friday 22 December 2006 10:31, Jan Beulich wrote:
> I object to the change to variable_test_bit() - why is that needed? I had
> specifically submitted a patch to get the const qualifier added there. Jan

The ADDR macro discards the const again.
I attached an alternative patch.


[-- Attachment #2: xen-cast-xen.diff --]
[-- Type: text/x-diff, Size: 785 bytes --]

diff -r f1ddc050d848 xen/common/gdbstub.c
--- a/xen/common/gdbstub.c	Thu Dec 21 20:44:35 2006 +0000
+++ b/xen/common/gdbstub.c	Fri Dec 22 09:51:42 2006 +0100
@@ -382,7 +382,7 @@ static int
 static int 
 process_command(struct cpu_user_regs *regs, struct gdb_context *ctx)
 {
-    char *ptr;
+    const char *ptr;
     unsigned long addr, length;
     int resume = 0;
 
diff -r f1ddc050d848 xen/include/asm-x86/bitops.h
--- a/xen/include/asm-x86/bitops.h	Thu Dec 21 20:44:35 2006 +0000
+++ b/xen/include/asm-x86/bitops.h	Fri Dec 22 10:39:57 2006 +0100
@@ -253,7 +253,7 @@ static __inline__ int variable_test_bit(
 	__asm__ __volatile__(
 		"btl %2,%1\n\tsbbl %0,%0"
 		:"=r" (oldbit)
-		:"m" (ADDR),"dIr" (nr));
+		:"m" (*(const volatile long *)addr),"dIr" (nr));
 	return oldbit;
 }
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] two missing bits (was Re: Add -Wcast-qual flag)

[Jan Beulich]

If this was with -Wconst-qual, yes. But didn't we agree that at least for now we'll
not add that warning (and hence we also don't absolutely have to change code
only affected by it)? Jan

>>> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> 22.12.06 10:40 >>>
Maybe we need a CONST_ADDR macro, as I suspect the problem is the cast
hidden inside the ADDR macro is dropping the const qualifier.

 -- Keir

On 22/12/06 9:31 am, "Jan Beulich" <jbeulich@novell.com> wrote:

> I object to the change to variable_test_bit() - why is that needed? I had
> specifically
> submitted a patch to get the const qualifier added there. Jan
> 
>>>> "Christoph Egger" <Christoph.Egger@amd.com> 22.12.06 10:05 >>>
> 
> Hi!
> 
> I catched two pieces with debug and crash_debug enabled.
> One of them fixes a build error even without the -Wcast-qual flag.
> 
> Patch attached.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com 
> http://lists.xensource.com/xen-devel 

Re: [PATCH] two missing bits (was Re: Add -Wcast-qual flag)

[Christoph Egger]

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

On Friday 22 December 2006 10:40, Keir Fraser wrote:
> Maybe we need a CONST_ADDR macro, as I suspect the problem is the cast
> hidden inside the ADDR macro is dropping the const qualifier.

That's right. Yet another alternative patch, this time with the CONST_ADDR 
macro.


>  -- Keir
>
> On 22/12/06 9:31 am, "Jan Beulich" <jbeulich@novell.com> wrote:
> > I object to the change to variable_test_bit() - why is that needed? I had
> > specifically
> > submitted a patch to get the const qualifier added there. Jan

[-- Attachment #2: xen-cast-xen.diff --]
[-- Type: text/x-diff, Size: 969 bytes --]

diff -r f1ddc050d848 xen/common/gdbstub.c
--- a/xen/common/gdbstub.c	Thu Dec 21 20:44:35 2006 +0000
+++ b/xen/common/gdbstub.c	Fri Dec 22 09:51:42 2006 +0100
@@ -382,7 +382,7 @@ static int
 static int 
 process_command(struct cpu_user_regs *regs, struct gdb_context *ctx)
 {
-    char *ptr;
+    const char *ptr;
     unsigned long addr, length;
     int resume = 0;
 
diff -r f1ddc050d848 xen/include/asm-x86/bitops.h
--- a/xen/include/asm-x86/bitops.h	Thu Dec 21 20:44:35 2006 +0000
+++ b/xen/include/asm-x86/bitops.h	Fri Dec 22 10:47:50 2006 +0100
@@ -22,6 +22,7 @@
  * used only when the constraint allows the operand to reside in a register.
  */
 
+#define CONST_ADDR (*(const volatile long *) addr)
 #define ADDR (*(volatile long *) addr)
 
 /**
@@ -253,7 +254,7 @@ static __inline__ int variable_test_bit(
 	__asm__ __volatile__(
 		"btl %2,%1\n\tsbbl %0,%0"
 		:"=r" (oldbit)
-		:"m" (ADDR),"dIr" (nr));
+		:"m" (CONST_ADDR),"dIr" (nr));
 	return oldbit;
 }
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] two missing bits (was Re: Add -Wcast-qual flag)

[Christoph Egger]

On Friday 22 December 2006 10:51, Jan Beulich wrote:
> If this was with -Wconst-qual, yes. But didn't we agree that at least for
> now we'll not add that warning (and hence we also don't absolutely have to
> change code only affected by it)? Jan

So far I understood, we agreed to not add the flag and not to add
the __UNCONST() hack.