Re: [Qemu-devel] [PATCH 1/6] fw_cfg: move initialisation of FWCfgState into instance_init

[Igor Mammedov]

On Sat, 10 Jun 2017 13:30:16 +0100
Mark Cave-Ayland <address@hidden> wrote:

> Signed-off-by: Mark Cave-Ayland <address@hidden>
> ---
>  hw/nvram/fw_cfg.c |   14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 316fca9..144e0c6 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -1017,6 +1017,15 @@ FWCfgState *fw_cfg_find(void)
>      return FW_CFG(object_resolve_path(FW_CFG_PATH, NULL));
>  }
>  
> +static void fw_cfg_init(Object *obj)
> +{
> +    FWCfgState *s = FW_CFG(obj);
> +
> +    s->entries[0] = g_new0(FWCfgEntry, fw_cfg_max_entry(s));
> +    s->entries[1] = g_new0(FWCfgEntry, fw_cfg_max_entry(s));
> +    s->entry_order = g_new0(int, fw_cfg_max_entry(s));
it doesn't seem right,

consider,
 object_new(fwcfg)
   1: fw_cfg_init -> g_new0(FWCfgEntry, fw_cfg_max_entry(s) -> 
FW_CFG_FILE_SLOTS_DFLT);
   2: set property x-file-slots
   3: realize -> fw_cfg_file_slots_allocate()

> @@ -1052,10 +1062,6 @@ static void fw_cfg_file_slots_allocate(FWCfgState *s, 
> Error **errp)
>                     file_slots_max);
>          return;
>      }
> -
> -    s->entries[0] = g_new0(FWCfgEntry, fw_cfg_max_entry(s));
> -    s->entries[1] = g_new0(FWCfgEntry, fw_cfg_max_entry(s));
> -    s->entry_order = g_new0(int, fw_cfg_max_entry(s));
opps, s->entries doesn't account for new values of x-file-slots

So question is why this patch is needed at all (it needs some reasoning in 
commit message)?

So for now NACK and I'd suggest to drop this patch.