[PATCH 11/19] virhash: Make sure that hash key is always copied

Peter Krempa posted 19 patches 6 years ago
[PATCH 11/19] virhash: Make sure that hash key is always copied
Posted by Peter Krempa 6 years ago
Fix all implementations of virHashKeyCopy to always return a valid
pointer.

Tweak the return value expectation comment so that it doesn't
necessarily require to allocate memory.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/conf/domain_addr.c | 5 +----
 src/util/virhash.c     | 4 +---
 src/util/virhash.h     | 3 ++-
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index f07b3d9725..e0be655772 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -997,10 +997,7 @@ virZPCIAddrKeyEqual(const void *namea,
 static void *
 virZPCIAddrKeyCopy(const void *name)
 {
-    unsigned int *copy;
-
-    if (VIR_ALLOC(copy) < 0)
-        return NULL;
+    unsigned int *copy = g_new0(unsigned int, 1);

     *copy = *((unsigned int *)name);
     return (void *)copy;
diff --git a/src/util/virhash.c b/src/util/virhash.c
index d5c5e017a1..c57d9f8292 100644
--- a/src/util/virhash.c
+++ b/src/util/virhash.c
@@ -94,9 +94,7 @@ static bool virHashStrEqual(const void *namea, const void *nameb)

 static void *virHashStrCopy(const void *name)
 {
-    char *ret;
-    ret = g_strdup(name);
-    return ret;
+    return g_strdup(name);
 }


diff --git a/src/util/virhash.h b/src/util/virhash.h
index 08f99d8a3d..143ce52206 100644
--- a/src/util/virhash.h
+++ b/src/util/virhash.h
@@ -83,7 +83,8 @@ typedef bool (*virHashKeyEqual)(const void *namea, const void *nameb);
  * Create a copy of the hash key, duplicating
  * memory allocation where applicable
  *
- * Returns a newly allocated copy of @name
+ * Returns a copy of @name which will eventually be passed to the
+ * 'virHashKeyFree' callback at the end of it's lifetime.
  */
 typedef void *(*virHashKeyCopy)(const void *name);
 /**
-- 
2.24.1

Re: [PATCH 11/19] virhash: Make sure that hash key is always copied
Posted by Ján Tomko 6 years ago
On Fri, Jan 31, 2020 at 03:31:15PM +0100, Peter Krempa wrote:
>Fix all implementations of virHashKeyCopy to always return a valid
>pointer.
>

Confusing.
VIR_ALLOC() < 0 is dead code already.

>Tweak the return value expectation comment so that it doesn't
>necessarily require to allocate memory.
>

It seems like this is the important part of the commit message
and should be in the summary.

>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/conf/domain_addr.c | 5 +----
> src/util/virhash.c     | 4 +---
> src/util/virhash.h     | 3 ++-
> 3 files changed, 4 insertions(+), 8 deletions(-)
>
>diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
>index f07b3d9725..e0be655772 100644
>--- a/src/conf/domain_addr.c
>+++ b/src/conf/domain_addr.c
>@@ -997,10 +997,7 @@ virZPCIAddrKeyEqual(const void *namea,
> static void *
> virZPCIAddrKeyCopy(const void *name)
> {
>-    unsigned int *copy;
>-
>-    if (VIR_ALLOC(copy) < 0)
>-        return NULL;
>+    unsigned int *copy = g_new0(unsigned int, 1);
>
>     *copy = *((unsigned int *)name);
>     return (void *)copy;
>diff --git a/src/util/virhash.c b/src/util/virhash.c
>index d5c5e017a1..c57d9f8292 100644
>--- a/src/util/virhash.c
>+++ b/src/util/virhash.c
>@@ -94,9 +94,7 @@ static bool virHashStrEqual(const void *namea, const void *nameb)
>
> static void *virHashStrCopy(const void *name)
> {
>-    char *ret;
>-    ret = g_strdup(name);
>-    return ret;
>+    return g_strdup(name);
> }
>
>

No functional change here either.

>diff --git a/src/util/virhash.h b/src/util/virhash.h
>index 08f99d8a3d..143ce52206 100644
>--- a/src/util/virhash.h
>+++ b/src/util/virhash.h
>@@ -83,7 +83,8 @@ typedef bool (*virHashKeyEqual)(const void *namea, const void *nameb);
>  * Create a copy of the hash key, duplicating
>  * memory allocation where applicable
>  *
>- * Returns a newly allocated copy of @name
>+ * Returns a copy of @name which will eventually be passed to the
>+ * 'virHashKeyFree' callback at the end of it's lifetime.

its

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano