Re: [PATCH 1/2] add macro REALLOCARRAY

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 1/2] add macro REALLOCARRAY

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:02:31

René Scharfe [off-list ref] writes:
The macro ALLOC_GROW manages several aspects of dynamic memory
allocations for arrays: It performs overprovisioning in order to avoid
reallocations in future calls, updates the allocation size variable,
multiplies the item size and thus allows users to simply specify the
item count, performs the reallocation and updates the array pointer.

Sometimes this is too much.  Add the macro REALLOCARRAY, which only
takes care of the latter three points and allows users to specify the
number of items an array can store directly.  It can increase and
also decrease its size.  Using this macro avoids duplicating the
array pointer name and takes care of item sizes automatically.

Signed-off-by: Rene Scharfe <redacted>
---
Makes sense.  Originally I had two minor gripes against this

 #1 a macro that modifies its arguments feels a bit too magical, and
    we would not want to overuse them.

 #2 REALLOC_ARRAY(array, size) would have been easier to read.

But #1 is shared with ALLOC_GROW(), and as long as we use it
everywhere (and by the looks of 2/2 we fairly widely do), readers
will get used to the pattern.  #2 still stands, though.

Thanks.
quoted hunk
 Documentation/technical/api-allocation-growing.txt | 3 +++
 git-compat-util.h                                  | 2 ++
 2 files changed, 5 insertions(+)
diff --git a/Documentation/technical/api-allocation-growing.txt b/Documentation/technical/api-allocation-growing.txt
index 542946b..4b5f049 100644
--- a/Documentation/technical/api-allocation-growing.txt
+++ b/Documentation/technical/api-allocation-growing.txt
@@ -34,3 +34,6 @@ item[nr++] = value you like;
 ------------
 
 You are responsible for updating the `nr` variable.
+
+If you need to specify the number of elements to allocate explicitly
+then use the macro `REALLOCARRAY(item, alloc)` instead of `ALLOC_GROW`.
diff --git a/git-compat-util.h b/git-compat-util.h
index 4e7e3f8..d926e4c 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -626,6 +626,8 @@ extern int odb_mkstemp(char *template, size_t limit, const char *pattern);
 extern int odb_pack_keep(char *name, size_t namesz, const unsigned char *sha1);
 extern char *xgetcwd(void);
 
+#define REALLOCARRAY(x, alloc) x = xrealloc((x), (alloc) * sizeof(*(x)))
+
 static inline size_t xsize_t(off_t len)
 {
 	if (len > (size_t) len)

Re: [PATCH 1/2] add macro REALLOCARRAY

From: Jeff King <hidden>
Date: 2016-06-15 23:02:32

On Mon, Sep 15, 2014 at 11:24:04AM -0700, Junio C Hamano wrote:
René Scharfe [off-list ref] writes:
quoted
The macro ALLOC_GROW manages several aspects of dynamic memory
allocations for arrays: It performs overprovisioning in order to avoid
reallocations in future calls, updates the allocation size variable,
multiplies the item size and thus allows users to simply specify the
item count, performs the reallocation and updates the array pointer.

Sometimes this is too much.  Add the macro REALLOCARRAY, which only
takes care of the latter three points and allows users to specify the
number of items an array can store directly.  It can increase and
also decrease its size.  Using this macro avoids duplicating the
array pointer name and takes care of item sizes automatically.

Signed-off-by: Rene Scharfe <redacted>
---
Makes sense.  Originally I had two minor gripes against this

 #1 a macro that modifies its arguments feels a bit too magical, and
    we would not want to overuse them.
This is probably getting into the too-magical territory, but I have long
considered a macro like:

  #define ALLOC(x) (x) = xmalloc(sizeof(*x))

to prevent obvious size-mismatch errors.  You could even call the macro
NEW() if you wanted to be really disgusting. :)

I rejected it as probably too cutesy (and non-idiomatic for experienced
C programmers), but I feel like this REALLOC_ARRAY is basically the same
thing. I dunno. It does make the code a bit more readable, once you
understand what the macro is doing.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help