[libvirt] [PATCH 06/14] secret: Use virSecretDefPtr rather than deref from virSecretObjPtr
Pavel Hrdina
phrdina at redhat.com
Tue Apr 25 08:36:17 UTC 2017
On Mon, Apr 24, 2017 at 02:00:15PM -0400, John Ferlan wrote:
> Rather than dereferencing obj->def->X, create a local 'def' variable
> variable that will dereference the def and use directly.
>
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
> src/conf/virsecretobj.c | 69 +++++++++++++++++++++++++++++++------------------
> 1 file changed, 44 insertions(+), 25 deletions(-)
>
> diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
> index 42f36c8..413955d 100644
> --- a/src/conf/virsecretobj.c
> +++ b/src/conf/virsecretobj.c
> @@ -139,8 +139,9 @@ static void
> virSecretObjDispose(void *opaque)
> {
> virSecretObjPtr obj = opaque;
> + virSecretDefPtr def = obj->def;
>
> - virSecretDefFree(obj->def);
> + virSecretDefFree(def);
Here it adds only a noise into the code, no actual improvement.
> if (obj->value) {
> /* Wipe before free to ensure we don't leave a secret on the heap */
> memset(obj->value, 0, obj->value_size);
> @@ -203,16 +204,18 @@ virSecretObjSearchName(const void *payload,
> const void *opaque)
> {
> virSecretObjPtr obj = (virSecretObjPtr) payload;
> + virSecretDefPtr def;
> struct virSecretSearchData *data = (struct virSecretSearchData *) opaque;
> int found = 0;
>
> virObjectLock(obj);
> + def = obj->def;
>
> - if (obj->def->usage_type != data->usageType)
> + if (def->usage_type != data->usageType)
> goto cleanup;
>
> if (data->usageType != VIR_SECRET_USAGE_TYPE_NONE &&
> - STREQ(obj->def->usage_id, data->usageID))
> + STREQ(def->usage_id, data->usageID))
> found = 1;
>
> cleanup:
> @@ -278,11 +281,13 @@ virSecretObjListRemove(virSecretObjListPtr secrets,
> virSecretObjPtr obj)
> {
> char uuidstr[VIR_UUID_STRING_BUFLEN];
> + virSecretDefPtr def;
>
> if (!obj)
> return;
> + def = obj->def;
>
> - virUUIDFormat(obj->def->uuid, uuidstr);
> + virUUIDFormat(def->uuid, uuidstr);
Same here,
> virObjectRef(obj);
> virObjectUnlock(obj);
>
> @@ -315,6 +320,7 @@ virSecretObjListAddLocked(virSecretObjListPtr secrets,
> virSecretDefPtr *oldDef)
> {
> virSecretObjPtr obj;
> + virSecretDefPtr def;
> virSecretObjPtr ret = NULL;
> char uuidstr[VIR_UUID_STRING_BUFLEN];
> char *configFile = NULL, *base64File = NULL;
> @@ -325,26 +331,27 @@ virSecretObjListAddLocked(virSecretObjListPtr secrets,
> /* Is there a secret already matching this UUID */
> if ((obj = virSecretObjListFindByUUIDLocked(secrets, newdef->uuid))) {
> virObjectLock(obj);
> + def = obj->def;
>
> - if (STRNEQ_NULLABLE(obj->def->usage_id, newdef->usage_id)) {
> - virUUIDFormat(obj->def->uuid, uuidstr);
> + if (STRNEQ_NULLABLE(def->usage_id, newdef->usage_id)) {
> + virUUIDFormat(def->uuid, uuidstr);
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("a secret with UUID %s is already defined for "
> "use with %s"),
> - uuidstr, obj->def->usage_id);
> + uuidstr, def->usage_id);
> goto cleanup;
> }
>
> - if (obj->def->isprivate && !newdef->isprivate) {
> + if (def->isprivate && !newdef->isprivate) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("cannot change private flag on existing secret"));
> goto cleanup;
> }
>
> if (oldDef)
> - *oldDef = obj->def;
> + *oldDef = def;
> else
> - virSecretDefFree(obj->def);
> + virSecretDefFree(def);
> obj->def = newdef;
> } else {
> /* No existing secret with same UUID,
> @@ -353,7 +360,8 @@ virSecretObjListAddLocked(virSecretObjListPtr secrets,
> newdef->usage_type,
> newdef->usage_id))) {
> virObjectLock(obj);
> - virUUIDFormat(obj->def->uuid, uuidstr);
> + def = obj->def;
> + virUUIDFormat(def->uuid, uuidstr);
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("a secret with UUID %s already defined for "
> "use with %s"),
> @@ -424,6 +432,7 @@ virSecretObjListGetHelper(void *payload,
> {
> struct virSecretObjListGetHelperData *data = opaque;
> virSecretObjPtr obj = payload;
> + virSecretDefPtr def;
>
> if (data->error)
> return 0;
> @@ -432,8 +441,9 @@ virSecretObjListGetHelper(void *payload,
> return 0;
>
> virObjectLock(obj);
> + def = obj->def;
>
> - if (data->filter && !data->filter(data->conn, obj->def))
> + if (data->filter && !data->filter(data->conn, def))
> goto cleanup;
>
> if (data->uuids) {
> @@ -444,7 +454,7 @@ virSecretObjListGetHelper(void *payload,
> goto cleanup;
> }
>
> - virUUIDFormat(obj->def->uuid, uuidstr);
> + virUUIDFormat(def->uuid, uuidstr);
> data->uuids[data->got] = uuidstr;
> }
>
> @@ -478,20 +488,22 @@ static bool
> virSecretObjMatchFlags(virSecretObjPtr obj,
> unsigned int flags)
> {
> + virSecretDefPtr def = obj->def;
> +
> /* filter by whether it's ephemeral */
> if (MATCH(VIR_CONNECT_LIST_SECRETS_FILTERS_EPHEMERAL) &&
> !((MATCH(VIR_CONNECT_LIST_SECRETS_EPHEMERAL) &&
> - obj->def->isephemeral) ||
> + def->isephemeral) ||
> (MATCH(VIR_CONNECT_LIST_SECRETS_NO_EPHEMERAL) &&
> - !obj->def->isephemeral)))
> + !def->isephemeral)))
> return false;
>
> /* filter by whether it's private */
> if (MATCH(VIR_CONNECT_LIST_SECRETS_FILTERS_PRIVATE) &&
> !((MATCH(VIR_CONNECT_LIST_SECRETS_PRIVATE) &&
> - obj->def->isprivate) ||
> + def->isprivate) ||
> (MATCH(VIR_CONNECT_LIST_SECRETS_NO_PRIVATE) &&
> - !obj->def->isprivate)))
> + !def->isprivate)))
> return false;
>
> return true;
> @@ -515,14 +527,16 @@ virSecretObjListPopulate(void *payload,
> {
> struct virSecretObjListData *data = opaque;
> virSecretObjPtr obj = payload;
> + virSecretDefPtr def;
> virSecretPtr secret = NULL;
>
> if (data->error)
> return 0;
>
> virObjectLock(obj);
> + def = obj->def;
>
> - if (data->filter && !data->filter(data->conn, obj->def))
> + if (data->filter && !data->filter(data->conn, def))
> goto cleanup;
>
> if (!virSecretObjMatchFlags(obj, data->flags))
> @@ -533,9 +547,9 @@ virSecretObjListPopulate(void *payload,
> goto cleanup;
> }
>
> - if (!(secret = virGetSecret(data->conn, obj->def->uuid,
> - obj->def->usage_type,
> - obj->def->usage_id))) {
> + if (!(secret = virGetSecret(data->conn, def->uuid,
> + def->usage_type,
> + def->usage_id))) {
> data->error = true;
> goto cleanup;
> }
> @@ -624,7 +638,9 @@ virSecretObjListGetUUIDs(virSecretObjListPtr secrets,
> int
> virSecretObjDeleteConfig(virSecretObjPtr obj)
> {
> - if (!obj->def->isephemeral &&
> + virSecretDefPtr def = obj->def;
> +
> + if (!def->isephemeral &&
here,
> unlink(obj->configFile) < 0 && errno != ENOENT) {
> virReportSystemError(errno, _("cannot unlink '%s'"),
> obj->configFile);
> @@ -653,10 +669,11 @@ virSecretObjDeleteData(virSecretObjPtr obj)
> int
> virSecretObjSaveConfig(virSecretObjPtr obj)
> {
> + virSecretDefPtr def = obj->def;
> char *xml = NULL;
> int ret = -1;
>
> - if (!(xml = virSecretDefFormat(obj->def)))
> + if (!(xml = virSecretDefFormat(def)))
> goto cleanup;
here,
> if (virFileRewriteStr(obj->configFile, S_IRUSR | S_IWUSR, xml) < 0)
> @@ -711,11 +728,12 @@ virSecretObjSetDef(virSecretObjPtr obj,
> unsigned char *
> virSecretObjGetValue(virSecretObjPtr obj)
> {
> + virSecretDefPtr def = obj->def;
> unsigned char *ret = NULL;
>
> if (!obj->value) {
> char uuidstr[VIR_UUID_STRING_BUFLEN];
> - virUUIDFormat(obj->def->uuid, uuidstr);
> + virUUIDFormat(def->uuid, uuidstr);
here,
> virReportError(VIR_ERR_NO_SECRET,
> _("secret '%s' does not have a value"), uuidstr);
> goto cleanup;
> @@ -735,6 +753,7 @@ virSecretObjSetValue(virSecretObjPtr obj,
> const unsigned char *value,
> size_t value_size)
> {
> + virSecretDefPtr def = obj->def;
> unsigned char *old_value, *new_value;
> size_t old_value_size;
>
> @@ -748,7 +767,7 @@ virSecretObjSetValue(virSecretObjPtr obj,
> obj->value = new_value;
> obj->value_size = value_size;
>
> - if (!obj->def->isephemeral && virSecretObjSaveData(obj) < 0)
> + if (!def->isephemeral && virSecretObjSaveData(obj) < 0)
> goto error;
and here.
Pavel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170425/ef25c049/attachment-0001.sig>
More information about the libvir-list
mailing list