Re: [Qemu-devel] [PATCH 2/6] qdev/compat: compat property infrastructure.

["Michael S. Tsirkin" <mst@redhat.com>]

Some coding style comments

On Mon, Jul 13, 2009 at 05:30:01PM +0200, Gerd Hoffmann wrote:
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/boards.h          |    1 +
>  hw/qdev-properties.c |   19 +++++++++++++++++++
>  hw/qdev.c            |    1 +
>  hw/qdev.h            |   11 +++++++++++
>  vl.c                 |    2 ++
>  5 files changed, 34 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/boards.h b/hw/boards.h
> index f6733b7..5a07d07 100644
> --- a/hw/boards.h
> +++ b/hw/boards.h
> @@ -17,6 +17,7 @@ typedef struct QEMUMachine {
>      int use_scsi;
>      int max_cpus;
>      int is_default;
> +    struct CompatProperty *compat_props;

use a typedef

>      struct QEMUMachine *next;
>  } QEMUMachine;
>  
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index ea937ae..2b1ee7d 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -244,3 +244,22 @@ void qdev_prop_set_defaults(DeviceState *dev, Property *props)
>      }
>  }
>  
> +static CompatProperty *compat_props;
> +
> +void qdev_register_compat_props(CompatProperty *props)
> +{
> +    compat_props = props;
> +}
> +
> +void qdev_prop_set_compat(DeviceState *dev)
> +{
> +    CompatProperty *prop;
> +
> +    if (!compat_props)

Missing {}

> +        return;
> +    for (prop = compat_props; prop->driver != NULL; prop++) {

!= NULL not needed in if

> +        if (strcmp(dev->info->name, prop->driver) != 0)

!= 0 not needed in if

> +            continue;

Missing {}

> +        qdev_prop_parse(dev, prop->property, prop->value);

check return value?

> +    }
> +}
> diff --git a/hw/qdev.c b/hw/qdev.c
> index cdb25d4..7ca078d 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -98,6 +98,7 @@ DeviceState *qdev_create(BusState *bus, const char *name)
>      dev->parent_bus = bus;
>      qdev_prop_set_defaults(dev, dev->info->props);
>      qdev_prop_set_defaults(dev, dev->parent_bus->info->props);
> +    qdev_prop_set_compat(dev);
>      LIST_INSERT_HEAD(&bus->children, dev, sibling);
>      return dev;
>  }
> diff --git a/hw/qdev.h b/hw/qdev.h
> index e807036..f49f641 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -8,6 +8,8 @@ typedef struct Property Property;
>  
>  typedef struct PropertyInfo PropertyInfo;
>  
> +typedef struct CompatProperty CompatProperty;
> +
>  typedef struct DeviceInfo DeviceInfo;
>  
>  typedef struct BusState BusState;
> @@ -71,6 +73,12 @@ struct PropertyInfo {
>      int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len);
>  };
>  
> +struct CompatProperty {
> +    const char *driver;
> +    const char *property;
> +    const char *value;
> +};
> +
>  /*** Board API.  This should go away once we have a machine config file.  ***/
>  
>  DeviceState *qdev_create(BusState *bus, const char *name);
> @@ -174,4 +182,7 @@ void qdev_prop_set_uint32(DeviceState *dev, const char *name, uint32_t value);
>  void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value);
>  void qdev_prop_set_defaults(DeviceState *dev, Property *props);
>  
> +void qdev_register_compat_props(CompatProperty *props);

qedev_set_compat_props might be a better name.

> +void qdev_prop_set_compat(DeviceState *dev);

qdev_parse_compat_props might be a better name.

> +
>  #endif
> diff --git a/vl.c b/vl.c
> index cdc7b8a..5402787 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -5911,6 +5911,8 @@ int main(int argc, char **argv, char **envp)
>  
>      module_call_init(MODULE_INIT_DEVICE);
>  
> +    if (machine->compat_props)

Missing {}

> +        qdev_register_compat_props(machine->compat_props);
>      machine->init(ram_size, boot_devices,
>                    kernel_filename, kernel_cmdline, initrd_filename, cpu_model);
>  
> -- 
> 1.6.2.5
> 
>