Re: [PATCH 34/36] machine: Use DEFINE_PROP_STRING for string properties

[Eduardo Habkost]

On Fri, Oct 30, 2020 at 06:10:34PM +0100, Paolo Bonzini wrote:
> On 29/10/20 23:02, Eduardo Habkost wrote:
> > +static Property machine_props[] = {
> > +    DEFINE_PROP_STRING("kernel", MachineState, kernel_filename),
> > +    DEFINE_PROP_STRING("initrd", MachineState, initrd_filename),
> > +    DEFINE_PROP_STRING("append", MachineState, kernel_cmdline),
> > +    DEFINE_PROP_STRING("dtb", MachineState, dtb),
> > +    DEFINE_PROP_STRING("dumpdtb", MachineState, dumpdtb),
> > +    DEFINE_PROP_STRING("dt-compatible", MachineState, dt_compatible),
> > +    DEFINE_PROP_STRING("firmware", MachineState, firmware),
> > +    DEFINE_PROP_STRING("memory-backend", MachineState, ram_memdev_id),
> > +    DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> 
> While I think generalizing the _code_ for static properties is obviously
> a good idea, I am not sure about generalizing the interface for adding them.
> 
> The reason is that we already have a place for adding properties in
> class_init, and having a second makes things "less local", so to speak.
> 
> What do you think about adding macros like
> 
>     OBJECT_CLASS_PROPERTY_ADD_STR(oc, MachineState, kernel_filename,
>                                   "kernel", prop_allow_set_always);

I like the idea of having an interface like this, but I would
like to avoid having to write even more boilerplate for each
property type to make this work.

What would you think of:

   OBJECT_CLASS_PROPERTY_ADD(oc,
       PROP_STRING("kernel", MachineState, kernel_filename),
       prop_allow_set_always);

Then we could make the same PROP_STRING macro usable both as
object_class_property_add_static() argument and as initializer
for existing static Property arrays.

-- 
Eduardo