[PATCH] Cleanup asm-x86/guest_access.h

[PATCH] Cleanup asm-x86/guest_access.h

[Christoph Egger]

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


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

[-- Attachment #2: xen_guest_access.diff --]
[-- Type: text/x-diff, Size: 4473 bytes --]

diff -r e66f047bc97e xen/include/asm-x86/guest_access.h
--- a/xen/include/asm-x86/guest_access.h	Tue Jan 09 18:56:44 2007 -0800
+++ b/xen/include/asm-x86/guest_access.h	Thu Jan 11 15:40:25 2007 +0100
@@ -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));              \
 })
 

[-- 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] Cleanup asm-x86/guest_access.h

[Petersson, Mats]

 

> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com 
> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of 
> Christoph Egger
> Sent: 11 January 2007 14:54
> To: xen-devel@lists.xensource.com
> 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.
> 
> 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
> 
> 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
> 

Re: [PATCH] Cleanup asm-x86/guest_access.h

[Keir Fraser]

On 12/1/07 12:01, "Petersson, Mats" <Mats.Petersson@amd.com> wrote:

>> 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

Re: [PATCH] Cleanup asm-x86/guest_access.h

[Christoph Egger]

On Friday 12 January 2007 14:37, Keir Fraser wrote:
> On 12/1/07 12:01, "Petersson, Mats" <Mats.Petersson@amd.com> wrote:
> >> 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.

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

Re: [PATCH] Cleanup asm-x86/guest_access.h

[Keir Fraser]

On 12/1/07 13:43, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

>> 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