Re: [Qemu-devel] [PATCH v2 1/5] fw_cfg: document fw_cfg_modify_iXX() update functions

[Peter Maydell]

On 31 August 2015 at 10:10, Marc Marí <address@hidden> wrote:
> From: "Gabriel L. Somlo" <address@hidden>
>
> Document the behavior of fw_cfg_modify_iXX() for leak-less updating
> of integer-type blobs.
>
> Currently only fw_cfg_modify_i16() is coded, but 32- and 64-bit versions
> may be added later if necessary..
>
> Signed-off-by: Gabriel Somlo <address@hidden>
> Signed-off-by: Gerd Hoffmann <address@hidden>
> Reviewed-by: Laszlo Ersek <address@hidden>
> ---
>  docs/specs/fw_cfg.txt | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
> index 74351dd..5bc7b96 100644
> --- a/docs/specs/fw_cfg.txt
> +++ b/docs/specs/fw_cfg.txt
> @@ -159,6 +159,17 @@ will convert a 16-, 32-, or 64-bit integer to 
> little-endian, then add
>  a dynamically allocated copy of the appropriately sized item to fw_cfg
>  under the given selector key value.
>
> +== fw_cfg_modify_iXX() ==
> +
> +Modify the value of an XX-bit item (where XX may be 16, 32, or 64).
> +Similarly to the corresponding fw_cfg_add_iXX() function set, convert
> +a 16-, 32-, or 64-bit integer to little endian, create a dynamically
> +allocated copy of the required size, and replace the existing item at
> +the given selector key value with the newly allocated one. The previous
> +item, assumed to have been allocated during an earlier call to
> +fw_cfg_add_iXX() or fw_cfg_modify_iXX() (of the same width XX), is freed
> +before the function returns.
> +
>  == fw_cfg_add_file() ==
>
>  Given a filename (i.e., fw_cfg item name), starting pointer, and size,

This doesn't cover fw_cfg_modify_file(); is that intentional?

As an aside, shouldn't this function-level documentation be done
via doc-comments in the header file where the prototypes are
declared? (You don't need to move the docs around in this series,
but it might be nice to do at some point.)

thanks
-- PMM