[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/3] target-i386: Add Intel HAX files
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH 2/3] target-i386: Add Intel HAX files |
Date: |
Wed, 9 Nov 2016 12:30:46 +0000 |
User-agent: |
Mutt/1.7.1 (2016-10-04) |
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.
signature.asc
Description: PGP signature
[Qemu-devel] [PATCH 1/3] kvm: move cpu synchronization code, Vincent Palatin, 2016/11/08
Re: [Qemu-devel] [PATCH 0/3] [RFC] Add HAX support, Paolo Bonzini, 2016/11/08