On Wed, Nov 9, 2016 at 1:30 PM, Stefan Hajnoczi <stefa...@gmail.com> wrote:
> 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.
My original plan was to import those files unmodified but this ship
has probably long-sailed.
Fixed in the v2 of this patch.
>
>> +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.
Found 2 instances by grepping and fixed them.
>
>> +//#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.
For the v2 of this patch, I will keep the macros and do your approach.
Then I will review all the debug print statements, see what I should
do with them in another patch (convert to tracing / remove).
