Re: [Xen-devel] [PATCH 10/18 V2]: PVH xen: introduce vmx_pvh.c and pvh.c

[Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>]

On Wed, 03 Apr 2013 09:06:18 +0100
"Jan Beulich" <JBeulich@xxxxxxxx> wrote:

> >>> On 03.04.13 at 03:37, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
> >>> wrote:
> > On Thu, 21 Mar 2013 16:49:12 +0000 Tim Deegan <tim@xxxxxxx> wrote:
> >> > +    uint acc_typ =
> >> > VMX_CONTROL_REG_ACCESS_TYPE(exit_qualification);
> >> > +    int cr, rc = 1;
> >> > +
> >> > +    switch ( acc_typ )
> >> > +    {
> >> > +        case VMX_CONTROL_REG_ACCESS_TYPE_MOV_TO_CR:
> >> > +        case VMX_CONTROL_REG_ACCESS_TYPE_MOV_FROM_CR:
> >> > +        {
> >> > +            uint gpr =
> >> > VMX_CONTROL_REG_ACCESS_GPR(exit_qualification);
> >> > +            uint64_t *regp = decode_register(gpr, regs, 0);
> >> > +            cr = VMX_CONTROL_REG_ACCESS_NUM(exit_qualification);
> >> > +
> >> > +            if (regp == NULL)
> >> > +                break;
> >> > +
> >> > +            /* pl don't embed switch statements */  <========
> >> > +            if (cr == 0)
> >> > +                rc = access_cr0(regs, acc_typ, regp);
> >> > +            else if (cr == 3) {
> >> > +                printk("PVH: d%d: unexpected cr3 access vmexit.
> >> > rip:%lx\n", 
> >> > +                       current->domain->domain_id, regs->rip);
> >> > +                domain_crash_synchronous();
> >> 
> >> Again, ITYM BUG().  And again. probably a switch statement.
> > 
> > No, lets not embed switch statements please, that is obfuscating.
> 
> I don't see a reason not to. Long rows of if/else-if are at least as
> obfuscating.

Not sure I agree. Long switch statements, as few places in xen code,
make it hard to figure which break goes with which switch, and that
makes code reading take longer.

But whatever. I'll change to switch. 

Mukesh

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