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

Re: [libvirt] [PATCH 03/10] secret: Add hashed virSecretObj and virSecretObjList



On 03/02/2016 11:55 AM, John Ferlan wrote:
> Add definitions and infrastucture required for a hashed virSecretObj
> and virSecretObjList including the class, object, lock setup, and
> disposal API's. Nothing will call these yet.
> 
> This infrastructure will replace the forward linked list logic
> within the secret_driver.
> 
> Signed-off-by: John Ferlan <jferlan redhat com>
> ---
>  src/conf/secret_conf.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  src/conf/secret_conf.h |   9 ++++
>  2 files changed, 123 insertions(+), 1 deletion(-)
> 

> +
> +static virClassPtr virSecretObjClass;
> +static void virSecretObjDispose(void *obj);
> +static virClassPtr virSecretObjListClass;
> +static void virSecretObjListDispose(void *obj);

I'm comparing against src/conf/network_conf.c, which lists the two
objects prior to the two function forward declarations (that is, swap
lines 2 and 3 of the above).

> +
> +struct _virSecretObjList {
> +    virObjectLockable parent;
> +
> +    /* uuid string -> virSecretObj  mapping
> +     * for O(1), lockless lookup-by-uuid */
> +    virHashTable *objs;
> +};

network_conf.c doesn't have the nice comment; but I like it here.

> +
> +static void
> +virSecretObjDispose(void *obj)
> +{
> +    virSecretObjPtr secret = obj;
> +
> +    virSecretDefFree(secret->def);
> +    if (secret->value != NULL) {

I might have dropped the '!= NULL', but that's cosmetic.

> +        memset(secret->value, 0, secret->value_size);
> +        VIR_FREE(secret->value);

Maybe worth a comment that we intentionally wipe before free, to
minimize the chance of leaving a no-longer-used secret on the heap?  The
name 'Secret' kind of makes it apparent why we spend the extra time
doing that, when most uses of VIR_FREE intentionally avoid the extra
wipe in the name of speed, but the comment may still be useful.

Addressing any of my comments shouldn't affect behavior, so:
ACK

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
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]