qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] qemu-img: sort block formats in help message


From: Mike Day
Subject: Re: [Qemu-devel] [PATCH] qemu-img: sort block formats in help message
Date: Mon, 5 May 2014 12:02:19 -0400

Thanks for the review! I've been able to shorten the next version quite a bit.

On Mon, May 5, 2014 at 9:42 AM, Stefan Hajnoczi <address@hidden> wrote:
>>  +static void GFunc_print_format(gpointer data, gpointer user)
>
> QEMU coding style is lowercase function and variable names.  The
> scripts/checkpatch.pl script should identify coding style violations,
> please run it.

Yeah this is ugly. I've long had checkpatch.pl configured as my
pre-commit hook. I've confirmed again this morning that these ugly
caps pass through with no error or warning. I'll look at the script.

>>  +
>>  +static void add_format_to_seq(void *opaque, const char *fmt_name)
>>  +{
>>  +    GSequence *seq = (GSequence *)opaque;
>>  +
>>  +    if (!g_sequence_lookup(seq, (gpointer)fmt_name,
>>  +                          compare_data, NULL)) {
>>  +        g_sequence_insert_sorted(seq, (gpointer)fmt_name,
>>  +                                 compare_data, NULL);
>>  +    }
>
> The type casts in this patch aren't necessary.  In C void* casts to and
> from any type without an explicit cast.  Only C++ demands explicit
> casts of void*.

The casts are ugly, and I don't know how to get rid of them here
beyond a pragma.  Due to the block and glib APIs I have to cast away a
const in the second parameter. I'm bracketed on both ends:
g_sequence_* needs that second parameter to be void * (pointer), while
the the block api (bdrv_iterate_format) defines the this function
pointer type as (*)(void *, const char *). Ideas?

Mike



reply via email to

[Prev in Thread] Current Thread [Next in Thread]