From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57092) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XoxVb-000525-OD for qemu-devel@nongnu.org; Thu, 13 Nov 2014 11:44:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XoxVU-0004EQ-8e for qemu-devel@nongnu.org; Thu, 13 Nov 2014 11:44:35 -0500 Received: from mail-by2on0147.outbound.protection.outlook.com ([207.46.100.147]:43488 helo=na01-by2-obe.outbound.protection.outlook.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XoxVT-0004EG-IL for qemu-devel@nongnu.org; Thu, 13 Nov 2014 11:44:28 -0500 From: Gary Hook Date: Thu, 13 Nov 2014 16:44:23 +0000 Message-ID: Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_D08A3C064232garyhooknimboxxcom_" MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 1/1] block migration: fix return value mismatch List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "qemu-devel@nongnu.org" --_000_D08A3C064232garyhooknimboxxcom_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable On 11/13/14, 6:46 AM, "Markus Armbruster" > wrote: Stefan Hajnoczi > writes: On Wed, Nov 12, 2014 at 06:48:18PM +0000, Gary Hook wrote: - return qemu_ftell(f) - last_ftell; + delta_ftell =3D qemu_ftell(f) - last_ftell; + return( (delta_ftell > 0) ? 1 : (delta_ftell < 0) ? -1 : 0 ); Good find! Please don't nest the ternary operator, it is hard to read. if (delta_ftell < 0) { return -1; } else if (delta_ftell > 0) { return 1; } else { return 0; } Bah, that's for wimps ;) return (delta_ftell > 0) - (delta_ftell < 0); Look ma, no branches! Ha-ha! Very good, but even less readable than the compressed ternary versio= n, IMO. This function only gets called once per migration; I don't see a few b= ranches as performance-critical and worth the sacrifice of clarity as to in= tent. --_000_D08A3C064232garyhooknimboxxcom_ Content-Type: text/html; charset="us-ascii" Content-ID: <596C7FC783A15E43852BB57D7DA81FC0@namprd02.prod.outlook.com> Content-Transfer-Encoding: quoted-printable
On 11/13/14, 6:46 = AM, "Markus Armbruster" <= armbru@redhat.com> wrote:

Stefan Hajnoczi <stefanha@gma= il.com> writes:

On Wed, Nov 12, 2014 at 06:48:18PM +0000, Gary Hook wrote:
-    return qemu_ftell(f) - last_ftell;
+    delta_ftell =3D qemu_ftell(f) - last_ftel= l;
+    return( (delta_ftell > 0) ? 1 : (delta= _ftell < 0) ? -1 : 0 );

Good find!

Please don't nest the ternary operator, it is hard to read.

if (delta_ftell < 0) {
     return -1;
} else if (delta_ftell > 0) {
     return 1;
} else {
     return 0;
}

Bah, that's for wimps ;)

    return (delta_ftell > 0) - (delta_ftell <= ; 0);

Look ma, no branches!

Ha-ha! Very good, = but even less readable than the compressed ternary version,
IMO. This function= only gets called once per migration; I don't see a few branches as perform= ance-critical and worth the sacrifice of clarity as to intent.

--_000_D08A3C064232garyhooknimboxxcom_--