Re: [Xen-devel] [PATCH 1/18 V2]: PVH xen: turn gdb_frames/gdt_ents into union

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.03.13 at 01:14, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> --- a/xen/include/public/arch-x86/xen.h
> +++ b/xen/include/public/arch-x86/xen.h
> @@ -170,7 +170,20 @@ struct vcpu_guest_context {
>      struct cpu_user_regs user_regs;         /* User-level CPU
> registers     */ struct trap_info trap_ctxt[256];        /* Virtual
> IDT                  */ unsigned long ldt_base, ldt_ents;       /* LDT
> (linear address, # ents) */ +#if __XEN_INTERFACE_VERSION__ < 0x00040300
>      unsigned long gdt_frames[16], gdt_ents; /* GDT (machine frames, #
> ents) */ +#else
> +    union {
> +        struct {
> +            /* GDT (machine frames, # ents) */
> +            unsigned long gdt_frames[16], gdt_ents;
> +        } pv;
> +        struct {
> +            /* PVH: GDTR addr and size */   
> +            unsigned long gdtaddr, gdtsz;
> +        } pvh;
> +    } u;

Leaving aside the line wrapping issue already pointed out by
others, I can only repeat that I don't see why you would name
the union as badly as "u" when the obvious name would be "gdt".

With that, I can further more only repeat that dropping the
"gdt_" and "gdt" prefixes on the names would be much preferred.

And finally I question the usefulness of having what is currently
named "gdtsz" be an "unsigned long" when this can't exceed a
16-bit quantity (the more if you used a limit value here rather
than a size, just like hardware does).

Jan


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