[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/7] security: Use virDomainSeclabelDefClear



On 01/12/2011 10:22 AM, Cole Robinson wrote:
> Renamed from virSecurityLabelDefFree.

I like the rename, but...

> 
> Signed-off-by: Cole Robinson <crobinso redhat com>
> ---
>  cfg.mk                           |    1 +
>  src/conf/domain_conf.c           |   17 +++++++++--------
>  src/conf/domain_conf.h           |    1 +
>  src/libvirt_private.syms         |    2 +-
>  src/qemu/qemu_driver.c           |    4 +---
>  src/security/security_apparmor.c |   11 ++---------
>  src/security/security_selinux.c  |    9 ++-------
>  7 files changed, 17 insertions(+), 28 deletions(-)
> 
> diff --git a/cfg.mk b/cfg.mk
> index 03186b3..2c6f595 100644
> --- a/cfg.mk
> +++ b/cfg.mk
> @@ -98,6 +98,7 @@ useless_free_options =				\
>    --name=virDomainInputDefFree			\
>    --name=virDomainNetDefFree			\
>    --name=virDomainObjFree			\
> +  --name=virDomainSeclabelDefClear		\

This is wrong.  It is _not_ a free-like function, because it does not
free the passed in pointer.  You probably want to have two functions:

/* clear guts only, must be given valid pointer */
void virDomainSeclabelDefClear(ptr) ATTRIBUTE_NONNULL(1);

/* clear entire allocated pointer, free-like */
void virDomainSeclabelDefFree(ptr) {
  if (!ptr) return;
  virDomainSeclabelDefClear(ptr);
  VIR_FREE(ptr);
}

and list only virDomainSeclabelDefFree in cfg.mk.

> -void virSecurityLabelDefFree(virDomainDefPtr def)
> +void virDomainSeclabelDefClear(virSecurityLabelDefPtr seclabel)
>  {
> -    VIR_FREE(def->seclabel.model);
> -    VIR_FREE(def->seclabel.label);
> -    VIR_FREE(def->seclabel.imagelabel);
> +    if (!seclabel)
> +        return;

which means this early exit should only be part of the Free variant, not
the Clear variant.

> +
> +    VIR_FREE(seclabel->model);
> +    VIR_FREE(seclabel->label);
> +    VIR_FREE(seclabel->imagelabel);
>  }
>  
>  static void
> @@ -855,7 +856,7 @@ void virDomainDefFree(virDomainDefPtr def)
>  
>      virDomainMemballoonDefFree(def->memballoon);
>  
> -    virSecurityLabelDefFree(def);
> +    virDomainSeclabelDefClear(&def->seclabel);

But hunks like this are okay (when a SeclabelDef occurs as an inline
struct member, rather than an indirect SeclabelDefPtr pointer).

> +++ b/src/libvirt_private.syms
> @@ -279,6 +279,7 @@ virDomainRemoveInactive;
>  virDomainSaveConfig;
>  virDomainSaveStatus;
>  virDomainSaveXML;
> +virDomainSeclabelDefClear;
>  virDomainSnapshotAssignDef;
>  virDomainSnapshotDefFormat;
>  virDomainSnapshotDefFree;
> @@ -313,7 +314,6 @@ virDomainWatchdogActionTypeToString;
>  virDomainWatchdogModelTypeFromString;
>  virDomainWatchdogModelTypeToString;
>  
> -
>  # domain_event.h

Why the whitespace change?

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]