[PATCH] fix public header hvm/save.h to be compatible with c++

[PATCH] fix public header hvm/save.h to be compatible with c++

[Ben Guthro]

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

Including save.h in a C++ application was throwing some errors, as it
was unhappy about the "new" keyword being used (even when wrapped in
an "extern C" block)

This change renames some variables to keep the compiler happy, as well
as casts the (void*) as appropriate.

Signed-off-by: Ben Guthro <ben@guthro.net>

diff --git a/xen/include/public/arch-x86/hvm/save.h
b/xen/include/public/arch-x86/hvm/save.h
index a82a5ee..20e5061 100644
--- a/xen/include/public/arch-x86/hvm/save.h
+++ b/xen/include/public/arch-x86/hvm/save.h
@@ -269,15 +269,15 @@ struct hvm_hw_cpu_compat {
 };

 static inline int _hvm_hw_fix_cpu(void *h) {
-    struct hvm_hw_cpu *new=h;
-    struct hvm_hw_cpu_compat *old=h;
+    struct hvm_hw_cpu *new_cpu = (struct hvm_hw_cpu *)h;
+    struct hvm_hw_cpu_compat *old_cpu = (struct hvm_hw_cpu_compat *)h;

     /* If we copy from the end backwards, we should
      * be able to do the modification in-place */
-    new->error_code=old->error_code;
-    new->pending_event=old->pending_event;
-    new->tsc=old->tsc;
-    new->msr_tsc_aux=0;
+    new_cpu->error_code=old_cpu->error_code;
+    new_cpu->pending_event=old_cpu->pending_event;
+    new_cpu->tsc=old_cpu->tsc;
+    new_cpu->msr_tsc_aux=0;

     return 0;
 }

[-- Attachment #2: public_headers_cpp_fixes.patch --]
[-- Type: application/octet-stream, Size: 929 bytes --]

diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h
index a82a5ee..20e5061 100644
--- a/xen/include/public/arch-x86/hvm/save.h
+++ b/xen/include/public/arch-x86/hvm/save.h
@@ -269,15 +269,15 @@ struct hvm_hw_cpu_compat {
 };
 
 static inline int _hvm_hw_fix_cpu(void *h) {
-    struct hvm_hw_cpu *new=h;
-    struct hvm_hw_cpu_compat *old=h;
+    struct hvm_hw_cpu *new_cpu = (struct hvm_hw_cpu *)h;
+    struct hvm_hw_cpu_compat *old_cpu = (struct hvm_hw_cpu_compat *)h;
 
     /* If we copy from the end backwards, we should
      * be able to do the modification in-place */
-    new->error_code=old->error_code;
-    new->pending_event=old->pending_event;
-    new->tsc=old->tsc;
-    new->msr_tsc_aux=0;
+    new_cpu->error_code=old_cpu->error_code;
+    new_cpu->pending_event=old_cpu->pending_event;
+    new_cpu->tsc=old_cpu->tsc;
+    new_cpu->msr_tsc_aux=0;
 
     return 0;
 }

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

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

Re: [PATCH] fix public header hvm/save.h to be compatible with c++

[Jan Beulich]

>>> On 05.10.12 at 14:05, Ben Guthro <ben@guthro.net> wrote:
> Including save.h in a C++ application was throwing some errors, as it
> was unhappy about the "new" keyword being used (even when wrapped in
> an "extern C" block)
> 
> This change renames some variables to keep the compiler happy, as well
> as casts the (void*) as appropriate.
> 
> Signed-off-by: Ben Guthro <ben@guthro.net>
> 
> diff --git a/xen/include/public/arch-x86/hvm/save.h
> b/xen/include/public/arch-x86/hvm/save.h
> index a82a5ee..20e5061 100644
> --- a/xen/include/public/arch-x86/hvm/save.h
> +++ b/xen/include/public/arch-x86/hvm/save.h
> @@ -269,15 +269,15 @@ struct hvm_hw_cpu_compat {
>  };
> 
>  static inline int _hvm_hw_fix_cpu(void *h) {
> -    struct hvm_hw_cpu *new=h;
> -    struct hvm_hw_cpu_compat *old=h;
> +    struct hvm_hw_cpu *new_cpu = (struct hvm_hw_cpu *)h;
> +    struct hvm_hw_cpu_compat *old_cpu = (struct hvm_hw_cpu_compat *)h;

While I don't mind the variable renames (albeit I'm questioning
whether the headers ought to be honoring the C++ reserved
names in the first place), I dislike the casts - they're redundant
in C (and it is my opinion that due to the mistakes casts can
hide they should be used as sparingly as possible), and at best
undesirable in C++ (you'd want to use static_cast<> there
instead).

Since the function is unused with __XEN__ undefined, I'd
instead suggest putting it inside a respective preprocessor
conditional.

>      /* If we copy from the end backwards, we should
>       * be able to do the modification in-place */
> -    new->error_code=old->error_code;
> -    new->pending_event=old->pending_event;
> -    new->tsc=old->tsc;
> -    new->msr_tsc_aux=0;
> +    new_cpu->error_code=old_cpu->error_code;
> +    new_cpu->pending_event=old_cpu->pending_event;
> +    new_cpu->tsc=old_cpu->tsc;
> +    new_cpu->msr_tsc_aux=0;

Once at it, you could have inserted the missing spaces around
the equal signs at once...

Jan

> 
>      return 0;
>  }

Re: [PATCH] fix public header hvm/save.h to be compatible with c++

[Ben Guthro]

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

Thanks for your review. Comments addressed, and re-tested.


Including save.h in a C++ application was throwing some errors, as it
was unhappy about the "new" keyword being used (even when wrapped in
an "extern C" block)

This change renames some variables, as well as wraps the function in a
__XEN__ preprocessor directive.

Signed-off-by: Ben Guthro <ben@guthro.net>

diff --git a/xen/include/public/arch-x86/hvm/save.h
b/xen/include/public/arch-x86/hvm/save.h
index a82a5ee..1b88c28 100644
--- a/xen/include/public/arch-x86/hvm/save.h
+++ b/xen/include/public/arch-x86/hvm/save.h
@@ -268,19 +268,21 @@ struct hvm_hw_cpu_compat {
     uint32_t error_code;
 };

+#ifdef __XEN__
 static inline int _hvm_hw_fix_cpu(void *h) {
-    struct hvm_hw_cpu *new=h;
-    struct hvm_hw_cpu_compat *old=h;
+    struct hvm_hw_cpu *new_cpu = h;
+    struct hvm_hw_cpu_compat *old_cpu = h;

     /* If we copy from the end backwards, we should
      * be able to do the modification in-place */
-    new->error_code=old->error_code;
-    new->pending_event=old->pending_event;
-    new->tsc=old->tsc;
-    new->msr_tsc_aux=0;
+    new_cpu->error_code = old_cpu->error_code;
+    new_cpu->pending_event = old_cpu->pending_event;
+    new_cpu->tsc = old_cpu->tsc;
+    new_cpu->msr_tsc_aux = 0;

     return 0;
 }
+#endif

 DECLARE_HVM_SAVE_TYPE_COMPAT(CPU, 2, struct hvm_hw_cpu, \
                              struct hvm_hw_cpu_compat, _hvm_hw_fix_cpu);

[-- Attachment #2: public_headers_cpp_fixes.patch --]
[-- Type: application/octet-stream, Size: 1073 bytes --]

diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h
index a82a5ee..1b88c28 100644
--- a/xen/include/public/arch-x86/hvm/save.h
+++ b/xen/include/public/arch-x86/hvm/save.h
@@ -268,19 +268,21 @@ struct hvm_hw_cpu_compat {
     uint32_t error_code;
 };
 
+#ifdef __XEN__
 static inline int _hvm_hw_fix_cpu(void *h) {
-    struct hvm_hw_cpu *new=h;
-    struct hvm_hw_cpu_compat *old=h;
+    struct hvm_hw_cpu *new_cpu = h;
+    struct hvm_hw_cpu_compat *old_cpu = h;
 
     /* If we copy from the end backwards, we should
      * be able to do the modification in-place */
-    new->error_code=old->error_code;
-    new->pending_event=old->pending_event;
-    new->tsc=old->tsc;
-    new->msr_tsc_aux=0;
+    new_cpu->error_code = old_cpu->error_code;
+    new_cpu->pending_event = old_cpu->pending_event;
+    new_cpu->tsc = old_cpu->tsc;
+    new_cpu->msr_tsc_aux = 0;
 
     return 0;
 }
+#endif
 
 DECLARE_HVM_SAVE_TYPE_COMPAT(CPU, 2, struct hvm_hw_cpu, \
                              struct hvm_hw_cpu_compat, _hvm_hw_fix_cpu);

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

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

Re: [PATCH] fix public header hvm/save.h to be compatible with c++

[Jan Beulich]

>>> On 05.10.12 at 14:46, Ben Guthro <ben@guthro.net> wrote:
> Thanks for your review. Comments addressed, and re-tested.

But now I fail to see why the rename is still necessary - you don't
write hypervisor code in C++, do you?

Jan

> Including save.h in a C++ application was throwing some errors, as it
> was unhappy about the "new" keyword being used (even when wrapped in
> an "extern C" block)
> 
> This change renames some variables, as well as wraps the function in a
> __XEN__ preprocessor directive.
> 
> Signed-off-by: Ben Guthro <ben@guthro.net>
> 
> diff --git a/xen/include/public/arch-x86/hvm/save.h
> b/xen/include/public/arch-x86/hvm/save.h
> index a82a5ee..1b88c28 100644
> --- a/xen/include/public/arch-x86/hvm/save.h
> +++ b/xen/include/public/arch-x86/hvm/save.h
> @@ -268,19 +268,21 @@ struct hvm_hw_cpu_compat {
>      uint32_t error_code;
>  };
> 
> +#ifdef __XEN__
>  static inline int _hvm_hw_fix_cpu(void *h) {
> -    struct hvm_hw_cpu *new=h;
> -    struct hvm_hw_cpu_compat *old=h;
> +    struct hvm_hw_cpu *new_cpu = h;
> +    struct hvm_hw_cpu_compat *old_cpu = h;
> 
>      /* If we copy from the end backwards, we should
>       * be able to do the modification in-place */
> -    new->error_code=old->error_code;
> -    new->pending_event=old->pending_event;
> -    new->tsc=old->tsc;
> -    new->msr_tsc_aux=0;
> +    new_cpu->error_code = old_cpu->error_code;
> +    new_cpu->pending_event = old_cpu->pending_event;
> +    new_cpu->tsc = old_cpu->tsc;
> +    new_cpu->msr_tsc_aux = 0;
> 
>      return 0;
>  }
> +#endif
> 
>  DECLARE_HVM_SAVE_TYPE_COMPAT(CPU, 2, struct hvm_hw_cpu, \
>                               struct hvm_hw_cpu_compat, _hvm_hw_fix_cpu);

Re: [PATCH] fix public header hvm/save.h to be compatible with c++

[Ben Guthro]

On Fri, Oct 5, 2012 at 9:13 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 05.10.12 at 14:46, Ben Guthro <ben@guthro.net> wrote:
>> Thanks for your review. Comments addressed, and re-tested.
>
> But now I fail to see why the rename is still necessary

I left the rename in there because you said you didn't mind them.
I was unsure if this meant you preferred the rename, or not. In the
interest of minimizing code churn to just the minimum - the ifdef
would be sufficient.
However, since you suggested I also fix the spacing around the equal
signs, I thought that the variable rename was in the same category,
for "code cleanup"

I think this is just a misunderstanding.

Would you like to see a third patch with just the ifdef?
Also - as a matter of procedure, should I reply to this original
thread when iterating on a patch like this, or start a new one, with
[PATCH v2] etc?

> - you don't write hypervisor code in C++, do you?
*shudders at the though* - no, most certainly not. This is someone
else's userspace daemon that I'm just integrating with new public
headers built against xen-unstable


Ben

> Jan
>
>> Including save.h in a C++ application was throwing some errors, as it
>> was unhappy about the "new" keyword being used (even when wrapped in
>> an "extern C" block)
>>
>> This change renames some variables, as well as wraps the function in a
>> __XEN__ preprocessor directive.
>>
>> Signed-off-by: Ben Guthro <ben@guthro.net>
>>
>> diff --git a/xen/include/public/arch-x86/hvm/save.h
>> b/xen/include/public/arch-x86/hvm/save.h
>> index a82a5ee..1b88c28 100644
>> --- a/xen/include/public/arch-x86/hvm/save.h
>> +++ b/xen/include/public/arch-x86/hvm/save.h
>> @@ -268,19 +268,21 @@ struct hvm_hw_cpu_compat {
>>      uint32_t error_code;
>>  };
>>
>> +#ifdef __XEN__
>>  static inline int _hvm_hw_fix_cpu(void *h) {
>> -    struct hvm_hw_cpu *new=h;
>> -    struct hvm_hw_cpu_compat *old=h;
>> +    struct hvm_hw_cpu *new_cpu = h;
>> +    struct hvm_hw_cpu_compat *old_cpu = h;
>>
>>      /* If we copy from the end backwards, we should
>>       * be able to do the modification in-place */
>> -    new->error_code=old->error_code;
>> -    new->pending_event=old->pending_event;
>> -    new->tsc=old->tsc;
>> -    new->msr_tsc_aux=0;
>> +    new_cpu->error_code = old_cpu->error_code;
>> +    new_cpu->pending_event = old_cpu->pending_event;
>> +    new_cpu->tsc = old_cpu->tsc;
>> +    new_cpu->msr_tsc_aux = 0;
>>
>>      return 0;
>>  }
>> +#endif
>>
>>  DECLARE_HVM_SAVE_TYPE_COMPAT(CPU, 2, struct hvm_hw_cpu, \
>>                               struct hvm_hw_cpu_compat, _hvm_hw_fix_cpu);
>
>
>

Re: [PATCH] fix public header hvm/save.h to be compatible with c++

[Jan Beulich]

>>> On 05.10.12 at 15:21, Ben Guthro <ben@guthro.net> wrote:
> On Fri, Oct 5, 2012 at 9:13 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 05.10.12 at 14:46, Ben Guthro <ben@guthro.net> wrote:
>>> Thanks for your review. Comments addressed, and re-tested.
>>
>> But now I fail to see why the rename is still necessary
> 
> I left the rename in there because you said you didn't mind them.
> I was unsure if this meant you preferred the rename, or not. In the
> interest of minimizing code churn to just the minimum - the ifdef
> would be sufficient.
> However, since you suggested I also fix the spacing around the equal
> signs, I thought that the variable rename was in the same category,
> for "code cleanup"
> 
> I think this is just a misunderstanding.
> 
> Would you like to see a third patch with just the ifdef?

I'll leave that decision to Keir, as it's going to be him to apply
(or at least ack) the change anyway.

> Also - as a matter of procedure, should I reply to this original
> thread when iterating on a patch like this, or start a new one, with
> [PATCH v2] etc?

Afaic, that's up to your preference.

Jan