On Thu, Jun 29, 2017 at 20:04:00 +0200, Andrea Bolognani wrote:
> Mostly style changes to make them a little bit nicer.
>
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
> src/conf/device_conf.c | 38 +++++++++++++++++++++++++++++++-------
> 1 file changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
> index 6ead830..facde0e 100644
> --- a/src/conf/device_conf.c
> +++ b/src/conf/device_conf.c
> @@ -45,21 +45,44 @@ virDomainDeviceInfoNew(void)
> return info;
> }
>
> +/**
> + * virDomainDeviceInfoClear:
> + * @info: device info
> + *
> + * Reset @info to its default state: all members will be set to their default
> + * value, and any memory associated with @info will be released. @info itself
> + * will still be valid after this function returns.
This is not true at this point:
@mastertype, @master, @rombar @bootIndex and @pciConnectFlags are not
touched here, so you still rely on some clearing beforehand.
This function is clearly meant just to release any allocated memory as
most of our Clear functions.
If you are going to sell it as a full-purposed cleaning functions, you
need to scrub everything.
> + *
> + * You only need to call this function if you're allocating a
> + * virDomainDeviceInfo on the stack or embedding one inside your own struct:
> + * virDomainDeviceInfoNew() already takes care of calling it for you.
> + */
> void
> virDomainDeviceInfoClear(virDomainDeviceInfoPtr info)
> {
> - VIR_FREE(info->alias);
> - memset(&info->addr, 0, sizeof(info->addr));
> info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;
> + memset(&info->addr, 0, sizeof(info->addr));
> +
> + VIR_FREE(info->alias);
> VIR_FREE(info->romfile);
> VIR_FREE(info->loadparm);
> }
>
> +/**
> + * virDomainDeviceInfoCopy:
> + * @dst: destination virDomainDeviceInfo
> + * @src: source virDomainDeviceInfo
> + *
> + * Perform a deep copy of @src into @dst.
> + *
> + * Return: 0 on success, <0 on failure
> + */
> int
> virDomainDeviceInfoCopy(virDomainDeviceInfoPtr dst,
> virDomainDeviceInfoPtr src)
> {
> - /* Assume that dst is already cleared */
> + /* Clear the destination */
> + virDomainDeviceInfoClear(dst);
e.g. this would be misleading, since it is not fully cleared ...
>
> /* first a shallow copy of *everything* */
> *dst = *src;
but thankfully fully overwritten ...
Rest looks good though
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list