On Tue, Sep 01, 2015 at 06:33:25PM +0100, Peter Maydell wrote: > On 31 August 2015 at 10:10, Marc Marí <mar...@redhat.com> wrote: > > From: "Gabriel L. Somlo" <so...@cmu.edu> > > > > 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 <so...@cmu.edu> > > Signed-off-by: Gerd Hoffmann <kra...@redhat.com> > > Reviewed-by: Laszlo Ersek <ler...@redhat.com> > > --- > > 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?
There should already be a paragraph in there (further down from where this is supposed to go in) describing fw_cfg_modify_file(), which isn't affected by this. > > 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.) You mean, leave docs/specs/fw_cfg.txt to deal with the guest-side port/mmio api only, and have the host-side functions simply documented as comments in include/hw/nvram/fw_cfg.h ? That should be relatively painless, if that's the agreed-upon convention... Thanks, --Gabriel