Skip to contentSkip to conversation

Cleanup asm-x86/guest_access.h

PATCH In review Christoph Egger 3 participants 5 messages Jan 11, 2007 – Jan 12, 2007

lore
Share & tools
lore

Christoph Egger

author
#0 lore raw

PATCH 1/1 Cleanup asm-x86/guest_access.h

Hi!

I cleaned up xen/include/asm-x86/guest_access.h. I also fixed a bug in __copy_field_from_guest(), which seems to me to
be a kind of cut-copy-paste bug.

copy_{to,from}_user() and copy_{to,from}_user_hvm() expect a non-const argument as the first argument. So I remove the consts from the initializer. I replace typeof with __typeof__, which is always available in gcc - it is not, when the -ansi option is used.

I do some explicit casts, which makes gcc errors more readable. In case, you pass an array by your mistake, which gets
casted to a char *, then gcc tells you that rather just "invalid initializer".

Christoph

xen/include/asm-x86/guest_access.h +17 -17
diff -r e66f047bc97e xen/include/asm-x86/guest_access.h
@@ -32,8 +32,8 @@
  * specifying an offset into the guest array.
  */
 #define copy_to_guest_offset(hnd, off, ptr, nr) ({      \
-    const typeof(ptr) _x = (hnd).p;                     \
-    const typeof(ptr) _y = (ptr);                       \
+    __typeof__(ptr) _x = (__typeof__(ptr))((hnd).p);    \
+    const __typeof__(ptr) _y = (__typeof__(ptr))(ptr);  \
     is_hvm_vcpu(current) ?                              \
     copy_to_user_hvm(_x+(off), _y, sizeof(*_x)*(nr)) :  \
     copy_to_user(_x+(off), _y, sizeof(*_x)*(nr));       \

@@ -44,8 +44,8 @@
  * specifying an offset into the guest array.
  */
 #define copy_from_guest_offset(ptr, hnd, off, nr) ({    \
-    const typeof(ptr) _x = (hnd).p;                     \
-    const typeof(ptr) _y = (ptr);                       \
+    const __typeof__(ptr) _x = (__typeof__(ptr))((hnd).p);\
+    __typeof__(ptr) _y = (__typeof__(ptr))(ptr);        \
     is_hvm_vcpu(current) ?                              \
     copy_from_user_hvm(_y, _x+(off), sizeof(*_x)*(nr)) :\
     copy_from_user(_y, _x+(off), sizeof(*_x)*(nr));     \

@@ -53,8 +53,8 @@
 
 /* Copy sub-field of a structure to guest context via a guest handle. */
 #define copy_field_to_guest(hnd, ptr, field) ({         \
-    const typeof(&(ptr)->field) _x = &(hnd).p->field;   \
-    const typeof(&(ptr)->field) _y = &(ptr)->field;     \
+    __typeof__(&(ptr)->field) _x = &(hnd).p->field;     \
+    const __typeof__(&(ptr)->field) _y = &(ptr)->field; \
     is_hvm_vcpu(current) ?                              \
     copy_to_user_hvm(_x, _y, sizeof(*_x)) :             \
     copy_to_user(_x, _y, sizeof(*_x));                  \

@@ -62,8 +62,8 @@
 
 /* Copy sub-field of a structure from guest context via a guest handle. */
 #define copy_field_from_guest(ptr, hnd, field) ({       \
-    const typeof(&(ptr)->field) _x = &(hnd).p->field;   \
-    const typeof(&(ptr)->field) _y = &(ptr)->field;     \
+    const __typeof__(&(ptr)->field) _x = &(hnd).p->field;\
+    __typeof__(&(ptr)->field) _y = &(ptr)->field;       \
     is_hvm_vcpu(current) ?                              \
     copy_from_user_hvm(_y, _x, sizeof(*_x)) :           \
     copy_from_user(_y, _x, sizeof(*_x));                \

@@ -78,34 +78,34 @@
      array_access_ok((hnd).p, (nr), sizeof(*(hnd).p)))
 
 #define __copy_to_guest_offset(hnd, off, ptr, nr) ({    \
-    const typeof(ptr) _x = (hnd).p;                     \
-    const typeof(ptr) _y = (ptr);                       \
+    __typeof__(ptr) _x = (__typeof__(ptr))((hnd).p);    \
+    const __typeof__(ptr) _y = (__typeof__(ptr))(ptr);  \
     is_hvm_vcpu(current) ?                              \
     copy_to_user_hvm(_x+(off), _y, sizeof(*_x)*(nr)) :  \
     __copy_to_user(_x+(off), _y, sizeof(*_x)*(nr));     \
 })
 
 #define __copy_from_guest_offset(ptr, hnd, off, nr) ({  \
-    const typeof(ptr) _x = (hnd).p;                     \
-    const typeof(ptr) _y = (ptr);                       \
+    const __typeof__(ptr) _x = (__typeof__(ptr))((hnd).p);\
+    __typeof__(ptr) _y = (__typeof__(ptr))(ptr);        \
     is_hvm_vcpu(current) ?                              \
     copy_from_user_hvm(_y, _x+(off),sizeof(*_x)*(nr)) : \
     __copy_from_user(_y, _x+(off), sizeof(*_x)*(nr));   \
 })
 
 #define __copy_field_to_guest(hnd, ptr, field) ({       \
-    const typeof(&(ptr)->field) _x = &(hnd).p->field;   \
-    const typeof(&(ptr)->field) _y = &(ptr)->field;     \
+    __typeof__(&(ptr)->field) _x = &(hnd).p->field;     \
+    const __typeof__(&(ptr)->field) _y = &(ptr)->field; \
     is_hvm_vcpu(current) ?                              \
     copy_to_user_hvm(_x, _y, sizeof(*_x)) :             \
     __copy_to_user(_x, _y, sizeof(*_x));                \
 })
 
 #define __copy_field_from_guest(ptr, hnd, field) ({     \
-    const typeof(&(ptr)->field) _x = &(hnd).p->field;   \
-    const typeof(&(ptr)->field) _y = &(ptr)->field;     \
+    const __typeof__(&(ptr)->field) _x = &(hnd).p->field;\
+    __typeof__(&(ptr)->field) _y = &(ptr)->field;       \
     is_hvm_vcpu(current) ?                              \
-    copy_from_user_hvm(_x, _y, sizeof(*_x)) :           \
+    copy_from_user_hvm(_y, _x, sizeof(*_x)) :           \
     __copy_from_user(_y, _x, sizeof(*_x));              \
 })

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#1 lore raw
Quoted 18 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Christoph Egger
Sent: 11 January 2007 14:54
To: [email protected]
Subject: [Xen-devel] [PATCH] Cleanup asm-x86/guest_access.h

Hi!

I cleaned up xen/include/asm-x86/guest_access.h. I also fixed a bug in
__copy_field_from_guest(), which seems to me to
be a kind of cut-copy-paste bug.

Quoted 6 lines

copy_{to,from}_user() and copy_{to,from}_user_hvm() expect a non-const argument as the first argument. So I remove the consts from the initializer.
I replace typeof with __typeof__, which is always available in gcc - it is
not, when the -ansi option is used.

And is it expected that Xen will EVER be compiled with -ansi? I think there's enough gcc-isms in other places that this will never happen - not that it really makes any difference, I'm just curious if anyone "cares"...

--
Mats

Quoted 8 lines

I do some explicit casts, which makes gcc errors more readable. In case, you pass an array by your mistake, which gets casted to a char *, then gcc tells you that rather just "invalid initializer".

Christoph

#2 lore raw
Quoted 11 lines · Mats
Quoted 6 lines

copy_{to,from}_user() and copy_{to,from}_user_hvm() expect a non-const argument as the first argument. So I remove the consts from the initializer.
I replace typeof with __typeof__, which is always available in gcc - it is
not, when the -ansi option is used.

And is it expected that Xen will EVER be compiled with -ansi? I think there's enough gcc-isms in other places that this will never happen - not that it really makes any difference, I'm just curious if anyone "cares"...

Yes, I'm going to take the const cleanups but bin the __typeof__ changes.

-- Keir

author
#3 lore raw
Quoted 8 lines · Keir Fraser
Quoted 7 lines · Mats
Quoted 6 lines

copy_{to,from}_user() and copy_{to,from}_user_hvm() expect a non-const argument as the first argument. So I remove the consts from the initializer.
I replace typeof with __typeof__, which is always available in gcc - it is
not, when the -ansi option is used.

Quoted 6 lines
Quoted 4 lines

And is it expected that Xen will EVER be compiled with -ansi? I think there's enough gcc-isms in other places that this will never happen - not that it really makes any difference, I'm just curious if anyone "cares"...

Yes, I'm going to take the const cleanups but bin the __typeof__ changes.

Do you also take the small bugfix in the __copy_field_from_guest() function at the very bottom in my patch ?

#4 lore raw
Quoted 4 lines · Christoph Egger
Quoted 1 line

Yes, I'm going to take the const cleanups but bin the __typeof__ changes.

Do you also take the small bugfix in the __copy_field_from_guest() function at the very bottom in my patch ?

Yes, thanks for that!

-- Keir