Re: [RFC PATCH 03/15] reftable: don't memset() a NULL from failed malloc()

From

René Scharfe <l.s.r@web.de>

Date

Jun 3, 2022, 22:22 UTC

Message-ID

<693ed3c3-535e-9eae-9fd7-ca612ebf6943@web.de>

In-Reply-To

<RFC-patch-03.15-0b570d112fc-20220603T183608Z-avarab@gmail.com>

Am 03.06.22 um 20:37 schrieb Ævar Arnfjörð Bjarmason:

> Return NULL instead of possibly feeding a NULL to memset() in
> reftable_calloc(). This issue was noted by GCC 12's -fanalyzer:
>
> 	reftable/publicbasics.c: In function ‘reftable_calloc’:
> 	reftable/publicbasics.c:43:9: error: use of possibly-NULL ‘p’ where non-null expected [CWE-690] [-Werror=analyzer-possible-null-argument]
> 	   43 |         memset(p, 0, sz);
> 	      |         ^~~~~~~~~~~~~~~~
> 	[...]
>
> This bug has been with us ever since this code was added in
> ef8a6c62687 (reftable: utility functions, 2021-10-07).
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  reftable/publicbasics.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/reftable/publicbasics.c b/reftable/publicbasics.c
> index 0ad7d5c0ff2..a18167f5ab7 100644
> --- a/reftable/publicbasics.c
> +++ b/reftable/publicbasics.c
> @@ -40,6 +40,8 @@ void reftable_free(void *p)
>  void *reftable_calloc(size_t sz)
>  {
>  	void *p = reftable_malloc(sz);
> +	if (!p)
> +		return NULL;
>  	memset(p, 0, sz);
>  	return p;
>  }

--- >8 ---
Subject: [PATCH] reftable: remove reftable_set_alloc()

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 reftable/basics.h          |  7 -------
 reftable/publicbasics.c    | 43 --------------------------------------
 reftable/reftable-malloc.h | 18 ----------------
 reftable/system.h          |  5 +++++
 4 files changed, 5 insertions(+), 68 deletions(-)
 delete mode 100644 reftable/reftable-malloc.h

diff --git a/reftable/basics.h b/reftable/basics.h
index 096b36862b..3386ff6484 100644
--- a/reftable/basics.h
+++ b/reftable/basics.h
@@ -46,13 +46,6 @@ int names_equal(char **a, char **b);
 /* returns the array size of a NULL-terminated array of strings. */
 int names_length(char **names);

-/* Allocation routines; they invoke the functions set through
- * reftable_set_alloc() */
-void *reftable_malloc(size_t sz);
-void *reftable_realloc(void *p, size_t sz);
-void reftable_free(void *p);
-void *reftable_calloc(size_t sz);
-
 /* Find the longest shared prefix size of `a` and `b` */
 struct strbuf;
 int common_prefix_size(struct strbuf *a, struct strbuf *b);
diff --git a/reftable/publicbasics.c b/reftable/publicbasics.c
index 0ad7d5c0ff..4dd4204ce6 100644
--- a/reftable/publicbasics.c
+++ b/reftable/publicbasics.c
@@ -6,52 +6,9 @@ license that can be found in the LICENSE file or at
 https://developers.google.com/open-source/licenses/bsd
 */

-#include "reftable-malloc.h"
-
 #include "basics.h"
 #include "system.h"

-static void *(*reftable_malloc_ptr)(size_t sz);
-static void *(*reftable_realloc_ptr)(void *, size_t);
-static void (*reftable_free_ptr)(void *);
-
-void *reftable_malloc(size_t sz)
-{
-	if (reftable_malloc_ptr)
-		return (*reftable_malloc_ptr)(sz);
-	return malloc(sz);
-}
-
-void *reftable_realloc(void *p, size_t sz)
-{
-	if (reftable_realloc_ptr)
-		return (*reftable_realloc_ptr)(p, sz);
-	return realloc(p, sz);
-}
-
-void reftable_free(void *p)
-{
-	if (reftable_free_ptr)
-		reftable_free_ptr(p);
-	else
-		free(p);
-}
-
-void *reftable_calloc(size_t sz)
-{
-	void *p = reftable_malloc(sz);
-	memset(p, 0, sz);
-	return p;
-}
-
-void reftable_set_alloc(void *(*malloc)(size_t),
-			void *(*realloc)(void *, size_t), void (*free)(void *))
-{
-	reftable_malloc_ptr = malloc;
-	reftable_realloc_ptr = realloc;
-	reftable_free_ptr = free;
-}
-
 int hash_size(uint32_t id)
 {
 	switch (id) {
diff --git a/reftable/reftable-malloc.h b/reftable/reftable-malloc.h
deleted file mode 100644
index 5f2185f1f3..0000000000
--- a/reftable/reftable-malloc.h
+++ /dev/null
@@ -1,18 +0,0 @@
-/*
-Copyright 2020 Google LLC
-
-Use of this source code is governed by a BSD-style
-license that can be found in the LICENSE file or at
-https://developers.google.com/open-source/licenses/bsd
-*/
-
-#ifndef REFTABLE_H
-#define REFTABLE_H
-
-#include <stddef.h>
-
-/* Overrides the functions to use for memory management. */
-void reftable_set_alloc(void *(*malloc)(size_t),
-			void *(*realloc)(void *, size_t), void (*free)(void *));
-
-#endif
diff --git a/reftable/system.h b/reftable/system.h
index 18f9207dfe..d7c7c20115 100644
--- a/reftable/system.h
+++ b/reftable/system.h
@@ -16,6 +16,11 @@ license that can be found in the LICENSE file or at
 #include "hash.h" /* hash ID, sizes.*/
 #include "dir.h" /* remove_dir_recursively, for tests.*/

+#define reftable_malloc(x) xmalloc(x)
+#define reftable_realloc(x, y) xrealloc((x), (y))
+#define reftable_free(x) free(x)
+#define reftable_calloc(x) xcalloc(1, (x))
+
 int hash_size(uint32_t id);

 #endif
--
2.35.3