Re: [PATCH] git-compat-util.h: introduce CALLOC(x)

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Dec 6, 2022, 01:58 UTC
Message-ID: <221206.86v8mpcmnt.gmgdl@evledraar.gmail.com>
In-Reply-To: <Y46fHhD3xionHgPE@coredump.intra.peff.net>

On Mon, Dec 05 2022, Jeff King wrote:

Show 22 quoted lines

> On Tue, Dec 06, 2022 at 12:12:32AM +0100, Ævar Arnfjörð Bjarmason wrote:
>
>> But if we *are* doing that then surely we should provide the full set of
>> functions. I.e. ALLOC() and ALLOC_ARRAY(), CALLOC() and CALLOC_ARRAY(),
>> and REALLOC() and REALLOC_ARRAY()?
>
> FWIW, I would be happy to see all of those (minus REALLOC(), as there is
> not really any point in growing or shrinking something with a fixed
> size).
>
> The biggest argument against them that I can see is that:
>
>   struct foo *x = malloc(sizeof(*x));
>
> is idiomatic C that newcomers to the project will easily understand,
> and:
>
>   struct foo *x;
>   ALLOC(x);
>
> is not. But it feels like we already crossed that bridge with
> ALLOC_ARRAY(), etc.

This is probably too ugly to exist, but if we are going to have more variants maybe one that would allow use within declarations would be better, e.g.:

	
	diff --git a/attr.c b/attr.c
	index 42ad6de8c7c..43ade426e57 100644
	--- a/attr.c
	+++ b/attr.c
	@@ -666,11 +666,10 @@ static void handle_attr_line(struct attr_stack *res,
	 
	 static struct attr_stack *read_attr_from_array(const char **list)
	 {
	-	struct attr_stack *res;
	+	struct attr_stack *INIT_CALLOC_ARRAY(res, 1);
	 	const char *line;
	 	int lineno = 0;
	 
	-	CALLOC_ARRAY(res, 1);
	 	while ((line = *(list++)) != NULL)
	 		handle_attr_line(res, line, "[builtin]", ++lineno,
	 				 READ_ATTR_MACRO_OK);
	diff --git a/git-compat-util.h b/git-compat-util.h
	index a76d0526f79..932d0907f3f 100644
	--- a/git-compat-util.h
	+++ b/git-compat-util.h
	@@ -1089,6 +1089,7 @@ int xstrncmpz(const char *s, const char *t, size_t len);
	 
	 #define ALLOC_ARRAY(x, alloc) (x) = xmalloc(st_mult(sizeof(*(x)), (alloc)))
	 #define CALLOC_ARRAY(x, alloc) (x) = xcalloc((alloc), sizeof(*(x)))
	+#define INIT_CALLOC_ARRAY(x, alloc) x = xcalloc((alloc), sizeof(*(x)))
	 #define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), st_mult(sizeof(*(x)), (alloc)))
	 
	 #define COPY_ARRAY(dst, src, n) copy_array((dst), (src), (n), sizeof(*(dst)) + \

I suspect (but haven't checked) that it might get us into the same parsing trouble as your UNUSED(name) macro.

Or maybe:
	
	diff --git a/attr.c b/attr.c
	index 42ad6de8c7c..c3cb5c98bbf 100644
	--- a/attr.c
	+++ b/attr.c
	@@ -669 +669 @@ static struct attr_stack *read_attr_from_array(const char **list)
	-	struct attr_stack *res;
	+	INIT_CALLOC_ARRAY(struct attr_stack *, res, 1);
	@@ -673 +672,0 @@ static struct attr_stack *read_attr_from_array(const char **list)
	-	CALLOC_ARRAY(res, 1);
	diff --git a/git-compat-util.h b/git-compat-util.h
	index a76d0526f79..fd9af571dc9 100644
	--- a/git-compat-util.h
	+++ b/git-compat-util.h
	@@ -1091,0 +1092 @@ int xstrncmpz(const char *s, const char *t, size_t len);
	+#define INIT_CALLOC_ARRAY(d, x, alloc) d x = xcalloc((alloc), sizeof(*(x)))
	
I think it's also worth considering just having the "normal C' versions,
but we could transform anything that doesn't look lik ea narrow
whitelist of patterns into an error with coccinelle.

I.e. if we're paranoid about "v" v.s. "*var" in the "sizeof" we could also check & tranform that with coccinelle...