Re: [Qemu-devel] [PATCH 1/2] fw_cfg: add fw_cfg_modify_i16 (update) method

[Gabriel L. Somlo]

On Mon, Jun 08, 2015 at 02:10:44PM -0400, Gabriel L. Somlo wrote:
> Allow the ability to modify the value of an existing 16-bit integer
> fw_cfg item.
> 
> Signed-off-by: Gabriel Somlo <address@hidden>
> ---
> 
> Couple of thoughts:
> 
>    1. I'm thinking about pre-emptively creating _i16, _i32, and _i64
>       versions, but right now (for fixing sparc and ppc) we only need
>       the _i16 version. What to do ?

Presumably, I'll update the documentation to mention these update
functions as well (one more reason to do all three versions rather
than just _i16) :)

> 
>    2. Part of me wants to find the memory location containing the previous
>       value and simply overwrite it, but I'll need to somehow ensure the
>       blob being replaced was of the same size, etc., which could get hairy.
>       So for now I'm going with the paranoid/safe version which allocates
>       a new blob and frees the old one.
> 
> Thanks,
>   Gabriel
> 
>  hw/nvram/fw_cfg.c         | 10 ++++++++++
>  include/hw/nvram/fw_cfg.h |  1 +
>  2 files changed, 11 insertions(+)
> 
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 68eff77..08b5cc3 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -484,6 +484,16 @@ void fw_cfg_add_i16(FWCfgState *s, uint16_t key, 
> uint16_t value)
>      fw_cfg_add_bytes(s, key, copy, sizeof(value));
>  }
>  
> +void fw_cfg_modify_i16(FWCfgState *s, uint16_t key, uint16_t value)
> +{
> +    uint16_t *copy, *old;
> +
> +    copy = g_malloc(sizeof(value));
> +    *copy = cpu_to_le16(value);
> +    old = fw_cfg_modify_bytes_read(s, key, copy, sizeof(value));
> +    g_free(old);
> +}
> +
>  void fw_cfg_add_i32(FWCfgState *s, uint16_t key, uint32_t value)
>  {
>      uint32_t *copy;
> diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
> index 6d8a8ac..bc6c4a0 100644
> --- a/include/hw/nvram/fw_cfg.h
> +++ b/include/hw/nvram/fw_cfg.h
> @@ -67,6 +67,7 @@ typedef void (*FWCfgReadCallback)(void *opaque, uint32_t 
> offset);
>  void fw_cfg_add_bytes(FWCfgState *s, uint16_t key, void *data, size_t len);
>  void fw_cfg_add_string(FWCfgState *s, uint16_t key, const char *value);
>  void fw_cfg_add_i16(FWCfgState *s, uint16_t key, uint16_t value);
> +void fw_cfg_modify_i16(FWCfgState *s, uint16_t key, uint16_t value);
>  void fw_cfg_add_i32(FWCfgState *s, uint16_t key, uint32_t value);
>  void fw_cfg_add_i64(FWCfgState *s, uint16_t key, uint64_t value);
>  void fw_cfg_add_callback(FWCfgState *s, uint16_t key, FWCfgCallback callback,
> -- 
> 2.1.0
>