From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:37208)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <benh@kernel.crashing.org>) id 1SXq0U-0003MX-1F
	for qemu-devel@nongnu.org; Fri, 25 May 2012 04:36:27 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <benh@kernel.crashing.org>) id 1SXq0N-000069-S7
	for qemu-devel@nongnu.org; Fri, 25 May 2012 04:36:21 -0400
Received: from gate.crashing.org ([63.228.1.57]:41240)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <benh@kernel.crashing.org>) id 1SXq0N-00005t-Hb
	for qemu-devel@nongnu.org; Fri, 25 May 2012 04:36:15 -0400
Message-ID: <1337934969.16119.18.camel@pasglop>
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Fri, 25 May 2012 18:36:09 +1000
In-Reply-To: <678ADF6E-BA35-48A4-865A-C45D1353022B@suse.de>
References: <1337585042.2779.4.camel@pasglop> <4FBA0F23.5040601@redhat.com>
	<1337594641.2779.59.camel@pasglop> <4FBA13DF.7040009@redhat.com>
	<1337918007.16119.13.camel@pasglop>
	<678ADF6E-BA35-48A4-865A-C45D1353022B@suse.de>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Mime-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH v2] spapr: Add "memop" hypercall
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Alexander Graf <agraf@suse.de>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>, kvm-ppc <kvm-ppc@vger.kernel.org>, "kvm@vger.kernel.org" <kvm@vger.kernel.org>, Avi Kivity <avi@redhat.com>

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

> > +    while (count--) {
> > +        switch (esize) {
> > +        case 0: tmp = ldub_phys(src);
> 
> I'm surprised checkpatch didn't complain here. Please do
> 
> 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.

Ben.

> Indentation?

Not sure what's up with identation, I had it all fixed up to please
checkpatch, maybe I screwed up the sending of the patch itself. Oh
well, I'm off to hospital on monday so that will have to wait til I'm
back (I regret you didn't make those comments on the previous iteration
of the patch though).

Ben.