Re: [PATCH] shared: avoid passing {NULL, 0} array to bsearch()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, May 19, 2023 at 10:41:08AM +0300, Dmitry Antipov wrote:
Fix the following warning reported by UBSan (as of gcc-13.1.1):

shared/hash.c:244:35: runtime error: null pointer passed as
argument 2, which is declared to never be null

Reviewed-by: Christophe Leroy <christophe.leroy@xxxxxxxxxx>
Signed-off-by: Dmitry Antipov <dmantipov@xxxxxxxxx>
---
shared/hash.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/shared/hash.c b/shared/hash.c
index 7fe3f80..f90e22d 100644
--- a/shared/hash.c
+++ b/shared/hash.c
@@ -241,12 +241,13 @@ void *hash_find(const struct hash *hash, const char *key)
		.key = key,
		.value = NULL
	};
-	const struct hash_entry *entry = bsearch(
-		&se, bucket->entries, bucket->used,
-		sizeof(struct hash_entry), hash_entry_cmp);
-	if (entry == NULL)
+	if (bucket->entries) {
+		const struct hash_entry *entry =
+			bsearch(&se, bucket->entries, bucket->used,
+				sizeof(struct hash_entry), hash_entry_cmp);
+		return entry ? (void *)entry->value : NULL;
+	} else

I'd avoid the unbalanced brackets and replace the bucket->entries
check with a return-early style. Would you be ok with me squashing this
into your patch?

diff --git a/shared/hash.c b/shared/hash.c
index f90e22d..a87bc50 100644
--- a/shared/hash.c
+++ b/shared/hash.c
@@ -241,13 +241,15 @@ void *hash_find(const struct hash *hash, const char *key)
 		.key = key,
 		.value = NULL
 	};
-	if (bucket->entries) {
-		const struct hash_entry *entry =
-			bsearch(&se, bucket->entries, bucket->used,
-				sizeof(struct hash_entry), hash_entry_cmp);
-		return entry ? (void *)entry->value : NULL;
-	} else
+	const struct hash_entry *entry;
+
+	if (!bucket->entries)
 		return NULL;
+
+	entry = bsearch(&se, bucket->entries, bucket->used,
+			sizeof(struct hash_entry), hash_entry_cmp);
+
+	return entry ? (void *)entry->value : NULL;
 }
int hash_del(struct hash *hash, const char *key)


However I'm curious about this *runtime* error you went through. Does it
have a backtrace? There are other places we call bsearch() passing
bucket->entries, but that should be an imposibble runtime situation
since we bail out on context creation if we can't create the hash table.

thanks
Lucas De Marchi



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux