From: Kirill Shchetiniuk <kshcheti@redhat.com>

Refactored the virSecretLookupParseSecret fucntion to use the
virXMLPropUUID fucntion, avoid getting the string and parsing it
later. Previously two separate error states merged into one by using
boolean NXOR operation.

Signed-off-by: Kirill Shchetiniuk <kshcheti@redhat.com>
---
 src/util/virsecret.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/src/util/virsecret.c b/src/util/virsecret.c
index 8a220a37ec..d85a563949 100644
--- a/src/util/virsecret.c
+++ b/src/util/virsecret.c
@@ -64,34 +64,27 @@ int
 virSecretLookupParseSecret(xmlNodePtr secretnode,
                            virSecretLookupTypeDef *def)
 {
-    g_autofree char *uuid = NULL;
     g_autofree char *usage = NULL;
+    int rc;
 
-    uuid = virXMLPropString(secretnode, "uuid");
     usage = virXMLPropString(secretnode, "usage");
-    if (uuid == NULL && usage == NULL) {
-        virReportError(VIR_ERR_XML_ERROR, "%s",
-                       _("missing secret uuid or usage attribute"));
+
+    if ((rc = virXMLPropUUID(secretnode, "uuid", VIR_XML_PROP_NONE, def->u.uuid)) < 0)
         return -1;
-    }
 
-    if (uuid && usage) {
+    if (!usage == (rc == 0)) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
                        _("either secret uuid or usage expected"));
         return -1;
     }
 
-    if (uuid) {
-        if (virUUIDParse(uuid, def->u.uuid) < 0) {
-            virReportError(VIR_ERR_XML_ERROR,
-                           _("invalid secret uuid '%1$s'"), uuid);
-            return -1;
-        }
+    if (rc > 0) {
         def->type = VIR_SECRET_LOOKUP_TYPE_UUID;
     } else {
         def->u.usage = g_steal_pointer(&usage);
         def->type = VIR_SECRET_LOOKUP_TYPE_USAGE;
     }
+
     return 0;
 }
 
-- 
2.49.0

On 7/22/25 17:12, Kirill Shchetiniuk via Devel wrote:
> From: Kirill Shchetiniuk <kshcheti@redhat.com>
> 
> Refactored the virSecretLookupParseSecret fucntion to use the
> virXMLPropUUID fucntion, avoid getting the string and parsing it
> later. Previously two separate error states merged into one by using
> boolean NXOR operation.
> 
> Signed-off-by: Kirill Shchetiniuk <kshcheti@redhat.com>
> ---
>  src/util/virsecret.c | 19 ++++++-------------
>  1 file changed, 6 insertions(+), 13 deletions(-)
> 
> diff --git a/src/util/virsecret.c b/src/util/virsecret.c
> index 8a220a37ec..d85a563949 100644
> --- a/src/util/virsecret.c
> +++ b/src/util/virsecret.c
> @@ -64,34 +64,27 @@ int
>  virSecretLookupParseSecret(xmlNodePtr secretnode,
>                             virSecretLookupTypeDef *def)
>  {
> -    g_autofree char *uuid = NULL;
>      g_autofree char *usage = NULL;
> +    int rc;
>  
> -    uuid = virXMLPropString(secretnode, "uuid");
>      usage = virXMLPropString(secretnode, "usage");
> -    if (uuid == NULL && usage == NULL) {
> -        virReportError(VIR_ERR_XML_ERROR, "%s",
> -                       _("missing secret uuid or usage attribute"));
> +
> +    if ((rc = virXMLPropUUID(secretnode, "uuid", VIR_XML_PROP_NONE, def->u.uuid)) < 0)
>          return -1;
> -    }
>  
> -    if (uuid && usage) {
> +    if (!usage == (rc == 0)) {

This works, but it's horrible to read. Since we already ...

>          virReportError(VIR_ERR_XML_ERROR, "%s",
>                         _("either secret uuid or usage expected"));
>          return -1;
>      }
>  
> -    if (uuid) {
> -        if (virUUIDParse(uuid, def->u.uuid) < 0) {
> -            virReportError(VIR_ERR_XML_ERROR,
> -                           _("invalid secret uuid '%1$s'"), uuid);
> -            return -1;
> -        }
> +    if (rc > 0) {

have different branches when "uuid" attribute was present or not, might
as well put corresponding checks here.

>          def->type = VIR_SECRET_LOOKUP_TYPE_UUID;
>      } else {
>          def->u.usage = g_steal_pointer(&usage);
>          def->type = VIR_SECRET_LOOKUP_TYPE_USAGE;
>      }
> +
>      return 0;
>  }
>  

Michal