secret: Introduce virSecretObjGetValue and virSecretObjGetValueSize

Introduce the final accessor's to _virSecretObject data and move the
structure from virsecretobj.h to virsecretobj.c

The virSecretObjSetValue logic will handle setting both the secret
value and the value_size. Some slight adjustments to the error path
over what was in secretSetValue were made.

Additionally, a slight logic change in secretGetValue where we'll
check for the internalFlags and error out before checking for
and erroring out for a NULL secret->value. That way, it won't be
obvious to anyone that the secret value wasn't set rather they'll
just know they cannot get the secret value since it's private.

src/conf/virsecretobj.c

@@ -36,6 +36,15 @@
 
 VIR_LOG_INIT("conf.virsecretobj");
 
+struct _virSecretObj {
+    virObjectLockable parent;
+    char *configFile;
+    char *base64File;
+    virSecretDefPtr def;
+    unsigned char *value;       /* May be NULL */
+    size_t value_size;
+};
+
 static virClassPtr virSecretObjClass;
 static virClassPtr virSecretObjListClass;
 static void virSecretObjDispose(void *obj);
@@ -755,6 +764,82 @@ virSecretObjSetDef(virSecretObjPtr secret,
 }
 
 
+unsigned char *
+virSecretObjGetValue(virSecretObjPtr secret)
+{
+    unsigned char *ret = NULL;
+
+    if (!secret->value) {
+        char uuidstr[VIR_UUID_STRING_BUFLEN];
+        virUUIDFormat(secret->def->uuid, uuidstr);
+        virReportError(VIR_ERR_NO_SECRET,
+                       _("secret '%s' does not have a value"), uuidstr);
+        goto cleanup;
+    }
+
+    if (VIR_ALLOC_N(ret, secret->value_size) < 0)
+        goto cleanup;
+    memcpy(ret, secret->value, secret->value_size);
+
+ cleanup:
+    return ret;
+}
+
+
+int
+virSecretObjSetValue(virSecretObjPtr secret,
+                     const unsigned char *value,
+                     size_t value_size)
+{
+    unsigned char *old_value, *new_value;
+    size_t old_value_size;
+
+    if (VIR_ALLOC_N(new_value, value_size) < 0)
+        return -1;
+
+    old_value = secret->value;
+    old_value_size = secret->value_size;
+
+    memcpy(new_value, value, value_size);
+    secret->value = new_value;
+    secret->value_size = value_size;
+
+    if (!secret->def->ephemeral && virSecretObjSaveData(secret) < 0)
+        goto error;
+
+    /* Saved successfully - drop old value */
+    if (old_value) {
+        memset(old_value, 0, old_value_size);
+        VIR_FREE(old_value);
+    }
+
+    return 0;
+
+ error:
+    /* Error - restore previous state and free new value */
+    secret->value = old_value;
+    secret->value_size = old_value_size;
+    memset(new_value, 0, value_size);
+    VIR_FREE(new_value);
+    return -1;
+}
+
+
+size_t
+virSecretObjGetValueSize(virSecretObjPtr secret)
+{
+    return secret->value_size;
+}
+
+
+void
+virSecretObjSetValueSize(virSecretObjPtr secret,
+                         size_t value_size)
+{
+    secret->value_size = value_size;
+}
+
+
 static int
 virSecretLoadValidateUUID(virSecretDefPtr def,
                           const char *file)

src/conf/virsecretobj.h

@@ -28,15 +28,6 @@
 
 typedef struct _virSecretObj virSecretObj;
 typedef virSecretObj *virSecretObjPtr;
-struct _virSecretObj {
-    virObjectLockable parent;
-    char *configFile;
-    char *base64File;
-    virSecretDefPtr def;
-    unsigned char *value;       /* May be NULL */
-    size_t value_size;
-};
-
 
 virSecretObjPtr virSecretObjNew(void);
 
@@ -105,6 +96,15 @@ virSecretDefPtr virSecretObjGetDef(virSecretObjPtr secret);
 
 void virSecretObjSetDef(virSecretObjPtr secret, virSecretDefPtr def);
 
+unsigned char *virSecretObjGetValue(virSecretObjPtr secret);
+
+int virSecretObjSetValue(virSecretObjPtr secret,
+                         const unsigned char *value, size_t value_size);
+
+size_t virSecretObjGetValueSize(virSecretObjPtr secret);
+
+void virSecretObjSetValueSize(virSecretObjPtr secret, size_t value_size);
+
 int virSecretLoadAllConfigs(virSecretObjListPtr secrets,
                             const char *configDir);
 #endif /* __VIRSECRETOBJ_H__ */

src/libvirt_private.syms

@@ -900,6 +900,8 @@ virSecretObjDeleteConfig;
 virSecretObjDeleteData;
 virSecretObjEndAPI;
 virSecretObjGetDef;
+virSecretObjGetValue;
+virSecretObjGetValueSize;
 virSecretObjListAdd;
 virSecretObjListExport;
 virSecretObjListFindByUsage;
@@ -911,6 +913,8 @@ virSecretObjListRemove;
 virSecretObjSaveConfig;
 virSecretObjSaveData;
 virSecretObjSetDef;
+virSecretObjSetValue;
+virSecretObjSetValueSize;
 
 
 # cpu/cpu.h

src/secret/secret_driver.c

@@ -312,16 +312,11 @@ secretSetValue(virSecretPtr obj,
                unsigned int flags)
 {
     int ret = -1;
-    unsigned char *old_value, *new_value;
-    size_t old_value_size;
     virSecretObjPtr secret;
     virSecretDefPtr def;
 
     virCheckFlags(0, -1);
 
-    if (VIR_ALLOC_N(new_value, value_size) < 0)
-        return -1;
-
     if (!(secret = secretObjFromSecret(obj)))
         goto cleanup;
 
@@ -329,40 +324,17 @@ secretSetValue(virSecretPtr obj,
     if (virSecretSetValueEnsureACL(obj->conn, def) < 0)
         goto cleanup;
 
-    old_value = secret->value;
-    old_value_size = secret->value_size;
-
-    memcpy(new_value, value, value_size);
-    secret->value = new_value;
-    secret->value_size = value_size;
-    if (!def->ephemeral) {
     if (secretEnsureDirectory() < 0)
         goto cleanup;
 
-        if (virSecretObjSaveData(secret) < 0)
-            goto restore_backup;
-    }
-    /* Saved successfully - drop old value */
-    if (old_value != NULL) {
-        memset(old_value, 0, old_value_size);
-        VIR_FREE(old_value);
-    }
-    new_value = NULL;
-
-    ret = 0;
+    if (virSecretObjSetValue(secret, value, value_size) < 0)
         goto cleanup;
 
- restore_backup:
-    /* Error - restore previous state and free new value */
-    secret->value = old_value;
-    secret->value_size = old_value_size;
-    memset(new_value, 0, value_size);
+    ret = 0;
 
  cleanup:
     virSecretObjEndAPI(&secret);
 
-    VIR_FREE(new_value);
-
     return ret;
 }
 
@@ -385,14 +357,6 @@ secretGetValue(virSecretPtr obj,
     if (virSecretGetValueEnsureACL(obj->conn, def) < 0)
         goto cleanup;
 
-    if (secret->value == NULL) {
-        char uuidstr[VIR_UUID_STRING_BUFLEN];
-        virUUIDFormat(obj->uuid, uuidstr);
-        virReportError(VIR_ERR_NO_SECRET,
-                       _("secret '%s' does not have a value"), uuidstr);
-        goto cleanup;
-    }
-
     if ((internalFlags & VIR_SECRET_GET_VALUE_INTERNAL_CALL) == 0 &&
         def->private) {
         virReportError(VIR_ERR_INVALID_SECRET, "%s",
@@ -400,10 +364,10 @@ secretGetValue(virSecretPtr obj,
         goto cleanup;
     }
 
-    if (VIR_ALLOC_N(ret, secret->value_size) < 0)
+    if (!(ret = virSecretObjGetValue(secret)))
         goto cleanup;
-    memcpy(ret, secret->value, secret->value_size);
-    *value_size = secret->value_size;
+
+    *value_size = virSecretObjGetValueSize(secret);
 
  cleanup:
     virSecretObjEndAPI(&secret);