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(a)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
Attachments:
- signature.asc (application/pgp-signature — 604 bytes)