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 <stefa...@redhat.com> 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
