Re: [Qemu-devel] [PATCH v2] spapr: Add "memop" hypercall

[Benjamin Herrenschmidt]

On Fri, 2012-05-25 at 10:54 +0200, Alexander Graf wrote:

> >> case x:
> >>    foo();
> >>    break();
> >> 
> >>> break;
> >>> +        case 1: tmp = lduw_phys(src); break;
> >>> +        case 2: tmp = ldl_phys(src);  break;
> >>> +        case 3: tmp = ldq_phys(src);  break;
> >>> +        default:
> >>> +        return H_PARAMETER;
> > 
> > Checkpatch absolutely complained and I decided to ignore it, seriously,
> > you really want to replace a nice & readable piece of code with
> > something that takes 3 pages and is generally gross & ugly ?
> > 
> > Some times, you have to ignore check patch and let sanity prevail.
> 
> I'm not all that keen on coding style rules. But check out
> arch/powerpc/kvm/emulate.c and tell me that it's a good idea to go
> with this "clean" approach. If you want it really clean, put the whole
> chunk above into a geberic helper that allows for everyone to say
> "read n bytes of data with native endianness into a u64". In that
> code, the more verbose coding style checkpatch suggests doesn't hurt
> and your function becomes even easier to read :)

I find your lack of taste disturbing Luke :-)

> Yeah, it's a shame I didn't read through it more thoroughly earlier - at 
> least it didn't take weeks in this round ;).
> 
> No worries though, if you can't make it until Monday, I'll fix it up myself 
> afterwards :). There's no black magic involved here,
> so I should be ok to respin myself.

Sure. No worries. To test it properly you really need a newer SLOF and
the patch to add -vga tho, I'll sort that out when I'm back.

Cheers,
Ben.

> Alex