From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:32798)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1c4S1l-0004F7-8p
	for qemu-devel@nongnu.org; Wed, 09 Nov 2016 07:30:54 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1c4S1i-0006Vb-4W
	for qemu-devel@nongnu.org; Wed, 09 Nov 2016 07:30:53 -0500
Received: from mail-wm0-x242.google.com ([2a00:1450:400c:c09::242]:35505)
	by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)
	(Exim 4.71) (envelope-from <stefanha@gmail.com>) id 1c4S1h-0006VK-U7
	for qemu-devel@nongnu.org; Wed, 09 Nov 2016 07:30:50 -0500
Received: by mail-wm0-x242.google.com with SMTP id a20so18171276wme.2
	for <qemu-devel@nongnu.org>; Wed, 09 Nov 2016 04:30:49 -0800 (PST)
Date: Wed, 9 Nov 2016 12:30:46 +0000
From: Stefan Hajnoczi <stefanha@gmail.com>
Message-ID: <20161109123046.GE4682@stefanha-x1.localdomain>
References: <cover.1478619442.git.vpalatin@chromium.org>
	<dc1f10ce25eab9d517675e0bdecfc6f4f65d77af.1478619442.git.vpalatin@chromium.org>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha1;
	protocol="application/pgp-signature"; boundary="ZInfyf7laFu/Kiw7"
Content-Disposition: inline
In-Reply-To: <dc1f10ce25eab9d517675e0bdecfc6f4f65d77af.1478619442.git.vpalatin@chromium.org>
Subject: Re: [Qemu-devel] [PATCH 2/3] target-i386: Add Intel HAX files
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: Vincent Palatin <vpalatin@chromium.org>
Cc: qemu-devel <qemu-devel@nongnu.org>


--ZInfyf7laFu/Kiw7
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Tue, Nov 08, 2016 at 04:39:28PM +0100, Vincent Palatin wrote:

Please run scripts/checkpatch.pl to verify that the code follows the
QEMU coding style.

> +hax_fd hax_host_open_vcpu(int vmid, int vcpuid)
> +{
> +    char *devfs_path = NULL;
> +    hax_fd fd;
> +
> +    devfs_path = hax_vcpu_devfs_string(vmid, vcpuid);
> +    if (!devfs_path) {
> +        fprintf(stderr, "Failed to get the devfs\n");
> +        return -EINVAL;
> +    }
> +
> +    fd = open(devfs_path, O_RDWR);
> +    qemu_vfree(devfs_path);

g_malloc(), g_new(), g_strdup(), etc must be matched with g_free(), not
qemu_vfree().  There are probably other instances of this issue in the
patches.

> +//#define DEBUG_HAX_SLOT
> +
> +#ifdef DEBUG_HAX_SLOT
> +#define DPRINTF(fmt, ...) \
> +    do { fprintf(stdout, fmt, ## __VA_ARGS__); } while (0)
> +#else
> +#define DPRINTF(fmt, ...) \
> +    do { } while (0)
> +#endif

Please consider using tracing instead of debug printfs.  See docs/tracing.txt.

If you really want to keep macros, please use:

#define DEBUG_HAX_SLOT 0
#define DPRINTF(fmt, ...) \
    do { \
        if (DEBUG_HAX_SLOT) { \
            fprintf(stdout, fmt, ## __VA_ARGS__); \
        } \
    } while (0)

This approach prevents bitrot because it allows the compiler to syntax
check the format string and arguments even when the printf is compiled
out.

--ZInfyf7laFu/Kiw7
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEcBAEBAgAGBQJYIxb2AAoJEJykq7OBq3PIJCsH/1hZElqiD0eNL+MxIbZzT05+
06WfFovkaxJuz85fhbi8LQ1ZcvkzfWtyjktvVxCouaof7lpG2M2EvKSK8hUFLG6x
vh9O80mWfA5YBDDKPbxEakeOO4SFFALrhdzEetauWEm3jJofdEg5FNu14dEy7hG0
LfNrOD+BkmzGq9fmb8jLT0N6XF0HXc6MJ8fzx6m++89lCeDFRxAvbXGt2uaNwqFO
KheEAunMISMBVzJMJ24NlWzdRQmngegBkkpajRtTEx4rqzwM4uTKiPDimSOTgNbU
2vJbQUyKV681DOWZk5sF81s+lDGGIMZAAHom735Ai5x2CgtOmbZvDq4MB99hK24=
=2hAX
-----END PGP SIGNATURE-----

--ZInfyf7laFu/Kiw7--