Re: [PATCH 01/14] include/hw: add helpers for defining versioned machine types

[Eric Blake]

On Wed, May 01, 2024 at 07:27:46PM +0100, Daniel P. Berrangé wrote:
> The various targets which define versioned machine types have
> a bunch of obfuscated macro code for defining unique function
> and variable names using string concatenation.
> 
> This addes a couple of helpers to improve the clarity of such
> code macro.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  include/hw/boards.h | 166 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 166 insertions(+)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 2fa800f11a..47ca450fca 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -414,6 +414,172 @@ struct MachineState {
>      struct NumaState *numa_state;
>  };
>  
> +/*
> + * The macros which follow are intended to facilitate the
> + * definition of versioned machine types, using a somewhat
> + * similar pattern across targets:
> + *
> + *  #define DEFINE_VIRT_MACHINE_IMPL(latest, ...) \
> + *      static void MACHINE_VER_SYM(class_init, virt, __VA_ARGS__)( \
> + *          ObjectClass *oc, \
> + *          void *data) \
> + *      { \
> + *          MachineClass *mc = MACHINE_CLASS(oc); \
> + *          MACHINE_VER_SYM(options, virt, __VA_ARGS__)(mc); \

Nice to include example usage of the macros.  __VA_ARGS__ is getting
expanded quite a few times here, but I don't see that as being too
much of a problem.

> + *  // Normal 2 digit eg 'virt-9.0'
> + *  #define DEFINE_VIRT_MACHINE(major, minor) \
> + *      DEFINE_VIRT_MACHINE_IMPL(false, major, minor)
> + *
> + *  // Bugfix 3 digit  eg 'virt-9.0.1'

Inconsistent on whether you are using one or two spaces before 'eg'.

> +
> +#define _MACHINE_VER_PICK(x1, x2, x3, x4, x5, x6, ...) x6

This helper macro is powerful; it may be worth a short comment, maybe
along the lines of:

/* Helper macro used to pick the right macro name based on the number
 * of extra arguments passed to the containing macro; usage:
 *
 *  _MACHINE_VER_PICK(__VA_ARGS__, \
 *                    MACRO_FOR_5_ARGS, \
 *                    MACRO_FOR_4_ARGS, \
 *                    MACRO_FOR_3_ARGS, \
 *                    MACRO_FOR_2_ARGS)(optional prefix args, __VA_ARGS__)
 */

But once understood, I see it comes in handy in several places below.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org