On 01/20/2016 11:55 AM, Markus Armbruster wrote: > Eric Blake writes: > >> As explained in the previous patches, matching argument order of >> 'name, &value' to JSON's "name":value makes sense. However, >> while the last two patches were easy with Coccinelle, I ended up >> doing this one all by hand. Now all the visitor callbacks match >> the main interface. >> >> Signed-off-by: Eric Blake >> Reviewed-by: Marc-André Lureau >> >> @@ -30,39 +30,42 @@ struct Visitor >> GenericList *(*next_list)(Visitor *v, GenericList **list, Error **errp); >> void (*end_list)(Visitor *v, Error **errp); >> >> - void (*type_enum)(Visitor *v, int *obj, const char * const strings[], >> - const char *kind, const char *name, Error **errp); >> + void (*type_enum)(Visitor *v, const char *name, int *obj, >> + const char * const strings[], const char *kind, > > Opportunity to change to 'const char *const'. I prefer that, because it > makes the fact that this is a pointer-* and not a binary operator-* > visually obvious. > > Same elsewhere. Hmm, I probably have churn later in the series. Will fix. >> /* May be NULL; most useful for input visitors. */ >> - void (*optional)(Visitor *v, bool *present, const char *name); >> + void (*optional)(Visitor *v, const char *name, bool *present); >> > > I checked the changes to this file carefully. Can we rely on the > compiler to flag mistakes in the rest of the patch? C's (intentionally-loose) treatment of 'char *' like 'void *' is a bit worrisome, but the fact that we have 'const' on only one of the two swapped arguments was indeed enough to make the compiler complain about mismatch in parameter types when trying to assign incorrectly-typed static functions to the updated struct members. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org