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@xxxxxxxxx>
> ---
> 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;
> }
We discussed this before, in
https://lore.kernel.org/git/RFC-patch-2.2-364d1194a95-20220415T101740Z-avarab@xxxxxxxxx/
If this code was actually used by Git and still not handling allocation
failures then I'd propose something like the below instead.
Next I'd probably try to convert reftable_calloc() calls to a variant
that takes size and count separately -- like calloc(3) does -- to avoid
unchecked multiplication.
--- >8 ---
Subject: [PATCH] reftable: remove reftable_set_alloc()
Use our native allocation functions for the reftable code to provide
consistent handling of allocation failures. Remove the ability to
change the allocator, as we don't need it anymore after this change.
Signed-off-by: René Scharfe <l.s.r@xxxxxx>
---
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
