[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH V3] libxc, libxenstore: make the headers C++-friendlier



>>> On 23.01.13 at 15:43, Razvan Cojocaru <rzvncj@xxxxxxxxx> wrote:
> --- a/xen/include/public/arch-x86/hvm/save.h  Tue Jan 22 09:33:10 2013 +0100
> +++ b/xen/include/public/arch-x86/hvm/save.h  Wed Jan 23 16:41:30 2013 +0200
> @@ -268,16 +268,18 @@ struct hvm_hw_cpu_compat {
>      uint32_t error_code;
>  };
>  
> +union hvm_hw_cpu_union {
> +    struct hvm_hw_cpu nat;
> +    struct hvm_hw_cpu_compat cmp;
> +};

I would think that specifically in C++ you don't want this type to
be visible at global scope. Please move it into the function below.

> +
>  static inline int _hvm_hw_fix_cpu(void *h) {
> -    struct hvm_hw_cpu *new=h;
> -    struct hvm_hw_cpu_compat *old=h;
> +    union hvm_hw_cpu_union *ucpu = (union hvm_hw_cpu_union *)h;
>  
> -    /* If we copy from the end backwards, we should
> -     * be able to do the modification in-place */

Don't drop this comment, please. The changes, as indicated earlier,
make it match actual behavior.

Also, just in case you aren't aware: Posting patches that touch both
tools and hypervisor files makes it more cumbersome to get them
applied (due to different people being responsible), so whenever
you can you should try to split patches accordingly - in the case here
it's obviously unnecessary to keep the pieces together in a single
patch.

Finally, from the very beginning the title of your patch was
suggesting the patch would touch tools code only. You should
avoid giving such false impressions. I wouldn't even have looked
at it (which you probably would have liked) if the rest of the
subject didn't get me curious.

Jan

> -    new->error_code=old->error_code;
> -    new->pending_event=old->pending_event;
> -    new->tsc=old->tsc;
> -    new->msr_tsc_aux=0;
> +    ucpu->nat.error_code = ucpu->cmp.error_code;
> +    ucpu->nat.pending_event = ucpu->cmp.pending_event;
> +    ucpu->nat.tsc = ucpu->cmp.tsc;
> +    ucpu->nat.msr_tsc_aux = 0;
>  
>      return 0;
>  }




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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.