add macro REALLOCARRAY
PATCH 1/2 add macro REALLOCARRAY
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.
--- Documentation/technical/api-allocation-growing.txt | 3 +++ git-compat-util.h | 2 ++ 2 files changed, 5 insertions(+)
Documentation/technical/api-allocation-growing.txt +3 -0
@@ -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`.
git-compat-util.h +2 -0
@@ -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)
Signature · 1 line
2.1.0
PATCH 2/2 use REALLOCARRAY for changing the allocation size of arrays
--- attr.c | 3 +-- builtin/apply.c | 2 +- builtin/for-each-ref.c | 9 +++------ builtin/index-pack.c | 4 +--- builtin/log.c | 2 +- builtin/merge.c | 2 +- builtin/mv.c | 8 ++++---- builtin/pack-objects.c | 3 +-- builtin/show-branch.c | 2 +- cache.h | 2 +- column.c | 6 ++---- commit-slab.h | 3 +-- fast-import.c | 2 +- git.c | 3 +-- graph.c | 14 ++++---------- khash.h | 12 ++++-------- line-log.c | 2 +- object.c | 2 +- pack-bitmap-write.c | 3 +-- pack-bitmap.c | 6 ++---- pack-objects.c | 3 +-- revision.c | 2 +- sh-i18n--envsubst.c | 5 +---- shallow.c | 3 +-- string-list.c | 3 +-- walker.c | 4 ++-- 26 files changed, 40 insertions(+), 70 deletions(-)
attr.c +1 -2
@@ -97,8 +97,7 @@ static struct git_attr *git_attr_internal(const char *name, int len) a->attr_nr = attr_nr++; git_attr_hash[pos] = a; - check_all_attr =xrealloc (check_all_attr, -sizeof (*check_all_attr) * attr_nr); +REALLOCARRAY (check_all_attr, attr_nr); check_all_attr[a->attr_nr].attr = a; check_all_attr[a->attr_nr].value =ATTR__UNKNOWN ;return a;
builtin/apply.c +1 -1
@@ -2626,7 +2626,7 @@ static void update_image(struct image *img,* NOTE: this knows that we never call remove_first_line() * on anything other than pre/post image. */ - img->line =xrealloc (img->line, nr *sizeof (*img->line)); +REALLOCARRAY (img->line, nr); img->line_allocated = img->line; }if (preimage_limit != postimage->nr)
builtin/for-each-ref.c +3 -6
@@ -138,10 +138,8 @@ static int parse_atom(const char *atom, const char *ep)/* Add it in, including the deref prefix */ at = used_atom_cnt; used_atom_cnt++; - used_atom =xrealloc (used_atom, - (sizeof *used_atom) * used_atom_cnt); - used_atom_type =xrealloc (used_atom_type, - (sizeof (*used_atom_type) * used_atom_cnt)); +REALLOCARRAY (used_atom, used_atom_cnt); +REALLOCARRAY (used_atom_type, used_atom_cnt); used_atom[at] =xmemdupz (atom, ep - atom); used_atom_type[at] = valid_atom[i].cmp_type;if (*atom =='*' )
@@ -870,8 +868,7 @@ static int grab_single_ref(const char *refname, const unsigned char *sha1, int f ref->flag = flag; cnt = cb->grab_cnt; - cb->grab_array =xrealloc (cb->grab_array, -sizeof (*cb->grab_array) * (cnt +1 )); +REALLOCARRAY (cb->grab_array, cnt +1 ); cb->grab_array[cnt++] = ref; cb->grab_cnt = cnt;return 0 ;
builtin/index-pack.c +1 -3
@@ -1140,9 +1140,7 @@ static void conclude_pack(int fix_thin_pack, const char *curr_pack, unsigned chaint nr_objects_initial = nr_objects;if (nr_unresolved <=0 )die (_ ("confusion beyond insanity" )); - objects =xrealloc (objects, - (nr_objects + nr_unresolved +1 ) - *sizeof (*objects)); +REALLOCARRAY (objects, nr_objects + nr_unresolved +1 );memset (objects + nr_objects +1 ,0 , nr_unresolved *sizeof (*objects)); f =sha1fd (output_fd, curr_pack);
builtin/log.c +1 -1
@@ -1440,7 +1440,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)continue ; nr++; - list =xrealloc (list, nr *sizeof (list[0 ])); +REALLOCARRAY (list, nr); list[nr -1 ] = commit; }if (nr ==0 )
builtin/merge.c +1 -1
@@ -556,7 +556,7 @@ static void parse_branch_merge_options(char *bmo)if (argc <0 )die (_ ("Bad branch.%s.mergeoptions string: %s" ), branch,split_cmdline_strerror (argc)); - argv =xrealloc (argv,sizeof (*argv) * (argc +2 )); +REALLOCARRAY (argv, argc +2 );memmove (argv +1 , argv,sizeof (*argv) * (argc +1 )); argc++; argv[0 ] ="branch.*.mergeoptions" ;
builtin/mv.c +4 -4
@@ -184,10 +184,10 @@ int cmd_mv(int argc, const char **argv, const char *prefix) modes[i] =WORKING_DIRECTORY ; n = argc + last - first; - source =xrealloc (source, n *sizeof (char *)); - destination =xrealloc (destination, n *sizeof (char *)); - modes =xrealloc (modes, n *sizeof (enum update_mode )); - submodule_gitfile =xrealloc (submodule_gitfile, n *sizeof (char *)); +REALLOCARRAY (source, n); +REALLOCARRAY (destination, n); +REALLOCARRAY (modes, n); +REALLOCARRAY (submodule_gitfile, n); dst =add_slash (dst); dst_len =strlen (dst);
builtin/pack-objects.c +1 -2
@@ -89,8 +89,7 @@ static void index_commit_for_bitmap(struct commit *commit) {if (indexed_commits_nr >= indexed_commits_alloc) { indexed_commits_alloc = (indexed_commits_alloc +32 ) *2 ; - indexed_commits =xrealloc (indexed_commits, - indexed_commits_alloc *sizeof (struct commit *)); +REALLOCARRAY (indexed_commits, indexed_commits_alloc); } indexed_commits[indexed_commits_nr++] = commit;
builtin/show-branch.c +1 -1
@@ -563,7 +563,7 @@ static int git_show_branch_config(const char *var, const char *value, void *cb) default_arg[default_num++] ="show-branch" ; }else if (default_alloc <= default_num +1 ) { default_alloc = default_alloc *3 /2 +20 ; - default_arg =xrealloc (default_arg,sizeof *default_arg * default_alloc); +REALLOCARRAY (default_arg, default_alloc); } default_arg[default_num++] =xstrdup (value); default_arg[default_num] =NULL ;
cache.h +1 -1
@@ -482,7 +482,7 @@ extern int daemonize(void); alloc = (nr); \ else \ alloc =alloc_nr (alloc); \ - x =xrealloc ((x), alloc *sizeof (*(x))); \ +REALLOCARRAY (x, alloc); \ } \ }while (0 )
column.c +2 -4
@@ -81,8 +81,7 @@ static void compute_column_width(struct column_data *data)*/ static void shrink_columns (struct column_data *data) { - data->width =xrealloc (data->width, -sizeof (*data->width) * data->cols); +REALLOCARRAY (data->width, data->cols);while (data->rows >1 ) {int x, total_width, cols, rows; rows = data->rows;
@@ -91,8 +90,7 @@ static void shrink_columns(struct column_data *data) data->rows--; data->cols =DIV_ROUND_UP (data->list->nr, data->rows);if (data->cols != cols) - data->width =xrealloc (data->width, -sizeof (*data->width) * data->cols); +REALLOCARRAY (data->width, data->cols);compute_column_width (data); total_width =strlen (data->opts.indent);
commit-slab.h +1 -2
@@ -90,8 +90,7 @@ static MAYBE_UNUSED elemtype *slabname## _at(struct slabname *s, \ \if (s->slab_count <= nth_slab) { \int i; \ - s->slab =xrealloc (s->slab, \ - (nth_slab +1 ) *sizeof (*s->slab)); \ +REALLOCARRAY (s->slab, nth_slab +1 ); \ stat_ ##slabname## realloc++; \ for (i = s->slab_count; i <= nth_slab; i++) \ s->slab[i] = NULL; \
fast-import.c +1 -1
@@ -878,7 +878,7 @@ static void start_packfile(void) pack_size =sizeof (hdr); object_count =0 ; - all_packs =xrealloc (all_packs,sizeof (*all_packs) * (pack_id +1 )); +REALLOCARRAY (all_packs, pack_id +1 ); all_packs[pack_id] = p; }
git.c +1 -2
@@ -282,8 +282,7 @@ static int handle_alias(int *argcp, const char ***argv)"trace: alias expansion: %s =>" , alias_command); - new_argv =xrealloc (new_argv,sizeof (char *) * - (count + *argcp)); +REALLOCARRAY (new_argv, count + *argcp);/* insert after command name */ memcpy (new_argv + count, *argv +1 ,sizeof (char *) * *argcp);
graph.c +4 -10
@@ -267,16 +267,10 @@ static void graph_ensure_capacity(struct git_graph *graph, int num_columns) graph->column_capacity *=2 ; }while (graph->column_capacity < num_columns); - graph->columns =xrealloc (graph->columns, -sizeof (struct column ) * - graph->column_capacity); - graph->new_columns =xrealloc (graph->new_columns, -sizeof (struct column ) * - graph->column_capacity); - graph->mapping =xrealloc (graph->mapping, -sizeof (int ) *2 * graph->column_capacity); - graph->new_mapping =xrealloc (graph->new_mapping, -sizeof (int ) *2 * graph->column_capacity); +REALLOCARRAY (graph->columns, graph->column_capacity); +REALLOCARRAY (graph->new_columns, graph->column_capacity); +REALLOCARRAY (graph->mapping, graph->column_capacity *2 ); +REALLOCARRAY (graph->new_mapping, graph->column_capacity *2 ); }/*
khash.h +4 -8
@@ -121,13 +121,9 @@ static const double __ac_HASH_UPPER = 0.77;if (!new_flags)return -1 ; \memset (new_flags,0xaa ,__ac_fsize (new_n_buckets) *sizeof (khint32_t)); \if (h->n_buckets < new_n_buckets) {/* expand */ \ -khkey_t *new_keys = (khkey_t *)xrealloc ((void *)h->keys, new_n_buckets *sizeof (khkey_t)); \ -if (!new_keys)return -1 ; \ - h->keys = new_keys; \ +REALLOCARRAY (h->keys, new_n_buckets); \if (kh_is_map) { \ -khval_t *new_vals = (khval_t *)xrealloc ((void *)h->vals, new_n_buckets *sizeof (khval_t)); \ -if (!new_vals)return -1 ; \ - h->vals = new_vals; \ +REALLOCARRAY (h->vals, new_n_buckets); \ } \ }/* otherwise shrink */ \ } \
@@ -160,8 +156,8 @@ static const double __ac_HASH_UPPER = 0.77; } \ } \if (h->n_buckets > new_n_buckets) {/* shrink the hash table */ \ - h->keys = (khkey_t *)xrealloc ((void *)h->keys, new_n_buckets *sizeof (khkey_t)); \ -if (kh_is_map) h->vals = (khval_t *)xrealloc ((void *)h->vals, new_n_buckets *sizeof (khval_t)); \ +REALLOCARRAY (h->keys, new_n_buckets); \ +if (kh_is_map)REALLOCARRAY (h->vals, new_n_buckets); \ } \free (h->flags);/* free the working space */ \ h->flags = new_flags; \
line-log.c +1 -1
@@ -533,7 +533,7 @@ static void fill_line_ends(struct diff_filespec *spec, long *lines, }/* shrink the array to fit the elements */ - ends =xrealloc (ends, cur *sizeof (*ends)); +REALLOCARRAY (ends, cur); *lines = cur-1 ; *line_ends = ends; }
object.c +1 -1
@@ -312,7 +312,7 @@ static void add_object_array_with_mode_context(struct object *obj, const char *nif (nr >= alloc) { alloc = (alloc +32 ) *2 ; - objects =xrealloc (objects, alloc *sizeof (*objects)); +REALLOCARRAY (objects, alloc); array->alloc = alloc; array->objects = objects; }
pack-bitmap-write.c +1 -2
@@ -111,8 +111,7 @@ static inline void push_bitmapped_commit(struct commit *commit, struct ewah_bitm {if (writer.selected_nr >= writer.selected_alloc) { writer.selected_alloc = (writer.selected_alloc +32 ) *2 ; - writer.selected =xrealloc (writer.selected, - writer.selected_alloc *sizeof (struct bitmapped_commit )); +REALLOCARRAY (writer.selected, writer.selected_alloc); } writer.selected[writer.selected_nr].commit = commit;
pack-bitmap.c +2 -4
@@ -400,10 +400,8 @@ static int ext_index_add_object(struct object *object, const char *name)if (hash_ret >0 ) {if (eindex->count >= eindex->alloc) { eindex->alloc = (eindex->alloc +16 ) *3 /2 ; - eindex->objects =xrealloc (eindex->objects, - eindex->alloc *sizeof (struct object *)); - eindex->hashes =xrealloc (eindex->hashes, - eindex->alloc *sizeof (uint32_t )); +REALLOCARRAY (eindex->objects, eindex->alloc); +REALLOCARRAY (eindex->hashes, eindex->alloc); } bitmap_pos = eindex->count;
pack-objects.c +1 -2
@@ -92,8 +92,7 @@ struct object_entry *packlist_alloc(struct packing_data *pdata,if (pdata->nr_objects >= pdata->nr_alloc) { pdata->nr_alloc = (pdata->nr_alloc +1024 ) *3 /2 ; - pdata->objects =xrealloc (pdata->objects, - pdata->nr_alloc *sizeof (*new_entry)); +REALLOCARRAY (pdata->objects, pdata->nr_alloc); } new_entry = pdata->objects + pdata->nr_objects++;
revision.c +1 -1
@@ -1397,7 +1397,7 @@ static void prepare_show_merge(struct rev_info *revs)continue ;if (ce_path_match (ce, &revs->prune_data,NULL )) { prune_num++; - prune =xrealloc (prune,sizeof (*prune) * prune_num); +REALLOCARRAY (prune, prune_num); prune[prune_num-2 ] = ce->name; prune[prune_num-1 ] =NULL ; }
sh-i18n--envsubst.c +1 -4
@@ -208,11 +208,8 @@ string_list_append (string_list_ty *slp, const char *s)/* Grow the list. */ if (slp->nitems >= slp->nitems_max) { -size_t nbytes; - slp->nitems_max = slp->nitems_max *2 +4 ; - nbytes = slp->nitems_max *sizeof (slp->item[0 ]); - slp->item = (const char **)xrealloc (slp->item, nbytes); +REALLOCARRAY (slp->item, slp->nitems_max); }/* Add the string to the end of the list. */
shallow.c +1 -2
@@ -392,8 +392,7 @@ static uint32_t *paint_alloc(struct paint_info *info)void *p;if (!info->slab_count || info->free + size > info->end) { info->slab_count++; - info->slab =xrealloc (info->slab, - info->slab_count *sizeof (*info->slab)); +REALLOCARRAY (info->slab, info->slab_count); info->free =xmalloc (COMMIT_SLAB_SIZE ); info->slab[info->slab_count -1 ] = info->free; info->end = info->free +COMMIT_SLAB_SIZE ;
string-list.c +1 -2
@@ -43,8 +43,7 @@ static int add_entry(int insert_at, struct string_list *list, const char *stringif (list->nr +1 >= list->alloc) { list->alloc +=32 ; - list->items =xrealloc (list->items, list->alloc - *sizeof (struct string_list_item )); +REALLOCARRAY (list->items, list->alloc); }if (index < list->nr) memmove(list->items + index + 1, list->items + index,
walker.c +2 -2
@@ -228,8 +228,8 @@ int walker_targets_stdin(char ***target, const char ***write_ref)if (targets >= targets_alloc) { targets_alloc = targets_alloc ? targets_alloc *2 :64 ; - *target =xrealloc (*target, targets_alloc *sizeof (**target)); - *write_ref =xrealloc (*write_ref, targets_alloc *sizeof (**write_ref)); +REALLOCARRAY (*target, targets_alloc); +REALLOCARRAY (*write_ref, targets_alloc); } (*target)[targets] =xstrdup (tg_one); (*write_ref)[targets] = rf_one ?xstrdup (rf_one) :NULL ;
Signature · 1 line
2.1.0
Quoted 6 lines · René Scharfe
The macro
ALLOC_GROWmanages 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.
Quoted 8 lines
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 <[email protected]>---
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 28 lines
Documentation/technical/api-allocation-growing.txt | 3 +++ git-compat-util.h | 2 ++ 2 files changed, 5 insertions(+)Documentation/technical/api-allocation-growing.txt +3
-0@@ -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`.git-compat-util.h +2
-0@@ -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)
Quoted 14 lines · Junio C Hamano
Quoted 12 lines · René Scharfe
The macro
ALLOC_GROWmanages 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.
Quoted 7 lines
Quoted 2 lines
Signed-off-by Rene Scharfe <[email protected]>---
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
Quoted 1 line · René Scharfe
+#define
REALLOCARRAY(x, alloc)x =xrealloc((x), (alloc) * sizeof(*(x)))
I have been wondering if "x" could be an expression that has an operator that binds weaker than the assignment '='. That may necessitate the LHS of the assignment to be somehow marked as bound the tightest, i.e.
#define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), (alloc) * sizeof(*(x)))
Or am I being overly silly?
Am 16.09.2014 um 05:04 schrieb Junio C Hamano:
Quoted 10 lines · René Scharfe
Quoted 1 line
+#define
REALLOCARRAY(x, alloc)x =xrealloc((x), (alloc) * sizeof(*(x)))I have been wondering if "x" could be an expression that has an operator that binds weaker than the assignment '='. That may necessitate the LHS of the assignment to be somehow marked as bound the tightest, i.e.
#define
REALLOC_ARRAY(x, alloc)(x) =xrealloc((x), (alloc) * sizeof(*(x)))Or am I being overly silly?
ALLOC_GROW did well without that. I can't think of a good use case for a complex expression on the right-hand side. That said, I think I still have a spare matching pair of parentheses lying around here somewhere, so let's play it safe and use them. :)
The added underscore is a good idea as well.
René
PATCH 1/2 add macro REALLOC_ARRAY
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 REALLOC_ARRAY, which only takes care of the latter three points and allows users to specfiy the number of items the array can store. It can increase and also decrease the size. Using the macro avoid duplicating the variable name and takes care of the item sizes automatically.
--- Documentation/technical/api-allocation-growing.txt | 3 +++ git-compat-util.h | 2 ++ 2 files changed, 5 insertions(+)
Documentation/technical/api-allocation-growing.txt +3 -0
@@ -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 `REALLOC_ARRAY(item, alloc)` instead of `ALLOC_GROW`.
git-compat-util.h +2 -0
@@ -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 REALLOC_ARRAY (x, alloc) (x) = xrealloc((x), (alloc) * sizeof(*(x))) +static inline size_t xsize_t (off_t len) {if (len > (size_t ) len)
Signature · 1 line
2.1.0
Re: [PATCH 1/2] add macro REALLOC_ARRAY
Quoted 11 lines · René Scharfe
The macro
ALLOC_GROWmanages 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
REALLOC_ARRAY, which only takes care of the latter three points and allows users to specfiy the number of items the array can store. It can increase and also decrease the size. Using the macro avoid duplicating the variable name and takes care of the item sizes automatically.
Is there a reason that ALLOC_GROW and REALLOC_ARRAY are defined in two separate header files (cache.h and git-compat-util.h, respectively)? It seems to me that they are close siblings and therefore I find it surprising that they are not defined right next to each other.
It's true that ALLOC_GROW contains a *tiny* bit of application logic in the form of the alloc_nr macro used to compute the new size of the array, but it's still pretty generic.
Michael
Signature · 2 lines
Michael Haggerty [email protected]
Re: [PATCH 1/2] add macro REALLOC_ARRAY
Am 24.09.2014 um 09:32 schrieb Michael Haggerty:
Quoted 4 lines
Is there a reason that
ALLOC_GROWandREALLOC_ARRAYare defined in two separate header files (cache.handgit-compat-util.h, respectively)? It seems to me that they are close siblings and therefore I find it surprising that they are not defined right next to each other.
REALLOC_ARRAY is more like xrealloc and it's used in places that only #include git-compat-util.h and not cache.h, so the first header was the right place.
ALLOC_GROW could be moved there in a separate patch, but I'm not sure it's worth it.
René
Re: [PATCH 1/2] add macro REALLOC_ARRAY
In reply to Michael HaggertyQuoted 13 lines · Michael Haggerty
Quoted 11 lines · René Scharfe
The macro
ALLOC_GROWmanages 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
REALLOC_ARRAY, which only takes care of the latter three points and allows users to specfiy the number of items the array can store. It can increase and also decrease the size. Using the macro avoid duplicating the variable name and takes care of the item sizes automatically.
Quoted 4 lines
Is there a reason that
ALLOC_GROWandREALLOC_ARRAYare defined in two separate header files (cache.handgit-compat-util.h, respectively)? It seems to me that they are close siblings and therefore I find it surprising that they are not defined right next to each other.
That was my initial reaction, but on the other hand, because REALLOC_ARRAY() should never be used on an array managed by ALLOC_GROW() without mucking with internal implementation details of what ALLOC_GROW() does yourself, there is not much reason for them to be listed and live together in the same place.
Listing them together might even invite confusion and mixed use by mistake.
PATCH 2/2 use REALLOC_ARRAY for changing the allocation size of arrays
In reply to René Scharfe--- attr.c | 3 +-- builtin/apply.c | 2 +- builtin/for-each-ref.c | 9 +++------ builtin/index-pack.c | 4 +--- builtin/log.c | 2 +- builtin/merge.c | 2 +- builtin/mv.c | 8 ++++---- builtin/pack-objects.c | 3 +-- builtin/show-branch.c | 2 +- cache.h | 2 +- column.c | 6 ++---- commit-slab.h | 3 +-- fast-import.c | 2 +- git.c | 3 +-- graph.c | 14 ++++---------- khash.h | 12 ++++-------- line-log.c | 2 +- object.c | 2 +- pack-bitmap-write.c | 3 +-- pack-bitmap.c | 6 ++---- pack-objects.c | 3 +-- revision.c | 2 +- sh-i18n--envsubst.c | 5 +---- shallow.c | 3 +-- string-list.c | 3 +-- walker.c | 4 ++-- 26 files changed, 40 insertions(+), 70 deletions(-)
attr.c +1 -2
@@ -97,8 +97,7 @@ static struct git_attr *git_attr_internal(const char *name, int len) a->attr_nr = attr_nr++; git_attr_hash[pos] = a; - check_all_attr =xrealloc (check_all_attr, -sizeof (*check_all_attr) * attr_nr); +REALLOC_ARRAY (check_all_attr, attr_nr); check_all_attr[a->attr_nr].attr = a; check_all_attr[a->attr_nr].value =ATTR__UNKNOWN ;return a;
builtin/apply.c +1 -1
@@ -2626,7 +2626,7 @@ static void update_image(struct image *img,* NOTE: this knows that we never call remove_first_line() * on anything other than pre/post image. */ - img->line =xrealloc (img->line, nr *sizeof (*img->line)); +REALLOC_ARRAY (img->line, nr); img->line_allocated = img->line; }if (preimage_limit != postimage->nr)
builtin/for-each-ref.c +3 -6
@@ -138,10 +138,8 @@ static int parse_atom(const char *atom, const char *ep)/* Add it in, including the deref prefix */ at = used_atom_cnt; used_atom_cnt++; - used_atom =xrealloc (used_atom, - (sizeof *used_atom) * used_atom_cnt); - used_atom_type =xrealloc (used_atom_type, - (sizeof (*used_atom_type) * used_atom_cnt)); +REALLOC_ARRAY (used_atom, used_atom_cnt); +REALLOC_ARRAY (used_atom_type, used_atom_cnt); used_atom[at] =xmemdupz (atom, ep - atom); used_atom_type[at] = valid_atom[i].cmp_type;if (*atom =='*' )
@@ -870,8 +868,7 @@ static int grab_single_ref(const char *refname, const unsigned char *sha1, int f ref->flag = flag; cnt = cb->grab_cnt; - cb->grab_array =xrealloc (cb->grab_array, -sizeof (*cb->grab_array) * (cnt +1 )); +REALLOC_ARRAY (cb->grab_array, cnt +1 ); cb->grab_array[cnt++] = ref; cb->grab_cnt = cnt;return 0 ;
builtin/index-pack.c +1 -3
@@ -1140,9 +1140,7 @@ static void conclude_pack(int fix_thin_pack, const char *curr_pack, unsigned chaint nr_objects_initial = nr_objects;if (nr_unresolved <=0 )die (_ ("confusion beyond insanity" )); - objects =xrealloc (objects, - (nr_objects + nr_unresolved +1 ) - *sizeof (*objects)); +REALLOC_ARRAY (objects, nr_objects + nr_unresolved +1 );memset (objects + nr_objects +1 ,0 , nr_unresolved *sizeof (*objects)); f =sha1fd (output_fd, curr_pack);
builtin/log.c +1 -1
@@ -1440,7 +1440,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)continue ; nr++; - list =xrealloc (list, nr *sizeof (list[0 ])); +REALLOC_ARRAY (list, nr); list[nr -1 ] = commit; }if (nr ==0 )
builtin/merge.c +1 -1
@@ -556,7 +556,7 @@ static void parse_branch_merge_options(char *bmo)if (argc <0 )die (_ ("Bad branch.%s.mergeoptions string: %s" ), branch,split_cmdline_strerror (argc)); - argv =xrealloc (argv,sizeof (*argv) * (argc +2 )); +REALLOC_ARRAY (argv, argc +2 );memmove (argv +1 , argv,sizeof (*argv) * (argc +1 )); argc++; argv[0 ] ="branch.*.mergeoptions" ;
builtin/mv.c +4 -4
@@ -184,10 +184,10 @@ int cmd_mv(int argc, const char **argv, const char *prefix) modes[i] =WORKING_DIRECTORY ; n = argc + last - first; - source =xrealloc (source, n *sizeof (char *)); - destination =xrealloc (destination, n *sizeof (char *)); - modes =xrealloc (modes, n *sizeof (enum update_mode )); - submodule_gitfile =xrealloc (submodule_gitfile, n *sizeof (char *)); +REALLOC_ARRAY (source, n); +REALLOC_ARRAY (destination, n); +REALLOC_ARRAY (modes, n); +REALLOC_ARRAY (submodule_gitfile, n); dst =add_slash (dst); dst_len =strlen (dst);
builtin/pack-objects.c +1 -2
@@ -89,8 +89,7 @@ static void index_commit_for_bitmap(struct commit *commit) {if (indexed_commits_nr >= indexed_commits_alloc) { indexed_commits_alloc = (indexed_commits_alloc +32 ) *2 ; - indexed_commits =xrealloc (indexed_commits, - indexed_commits_alloc *sizeof (struct commit *)); +REALLOC_ARRAY (indexed_commits, indexed_commits_alloc); } indexed_commits[indexed_commits_nr++] = commit;
builtin/show-branch.c +1 -1
@@ -563,7 +563,7 @@ static int git_show_branch_config(const char *var, const char *value, void *cb) default_arg[default_num++] ="show-branch" ; }else if (default_alloc <= default_num +1 ) { default_alloc = default_alloc *3 /2 +20 ; - default_arg =xrealloc (default_arg,sizeof *default_arg * default_alloc); +REALLOC_ARRAY (default_arg, default_alloc); } default_arg[default_num++] =xstrdup (value); default_arg[default_num] =NULL ;
cache.h +1 -1
@@ -482,7 +482,7 @@ extern int daemonize(void); alloc = (nr); \ else \ alloc =alloc_nr (alloc); \ - x =xrealloc ((x), alloc *sizeof (*(x))); \ +REALLOC_ARRAY (x, alloc); \ } \ }while (0 )
column.c +2 -4
@@ -81,8 +81,7 @@ static void compute_column_width(struct column_data *data)*/ static void shrink_columns (struct column_data *data) { - data->width =xrealloc (data->width, -sizeof (*data->width) * data->cols); +REALLOC_ARRAY (data->width, data->cols);while (data->rows >1 ) {int x, total_width, cols, rows; rows = data->rows;
@@ -91,8 +90,7 @@ static void shrink_columns(struct column_data *data) data->rows--; data->cols =DIV_ROUND_UP (data->list->nr, data->rows);if (data->cols != cols) - data->width =xrealloc (data->width, -sizeof (*data->width) * data->cols); +REALLOC_ARRAY (data->width, data->cols);compute_column_width (data); total_width =strlen (data->opts.indent);
commit-slab.h +1 -2
@@ -90,8 +90,7 @@ static MAYBE_UNUSED elemtype *slabname## _at(struct slabname *s, \ \if (s->slab_count <= nth_slab) { \int i; \ - s->slab =xrealloc (s->slab, \ - (nth_slab +1 ) *sizeof (*s->slab)); \ +REALLOC_ARRAY (s->slab, nth_slab +1 ); \ stat_ ##slabname## realloc++; \ for (i = s->slab_count; i <= nth_slab; i++) \ s->slab[i] = NULL; \
fast-import.c +1 -1
@@ -878,7 +878,7 @@ static void start_packfile(void) pack_size =sizeof (hdr); object_count =0 ; - all_packs =xrealloc (all_packs,sizeof (*all_packs) * (pack_id +1 )); +REALLOC_ARRAY (all_packs, pack_id +1 ); all_packs[pack_id] = p; }
git.c +1 -2
@@ -282,8 +282,7 @@ static int handle_alias(int *argcp, const char ***argv)"trace: alias expansion: %s =>" , alias_command); - new_argv =xrealloc (new_argv,sizeof (char *) * - (count + *argcp)); +REALLOC_ARRAY (new_argv, count + *argcp);/* insert after command name */ memcpy (new_argv + count, *argv +1 ,sizeof (char *) * *argcp);
graph.c +4 -10
@@ -267,16 +267,10 @@ static void graph_ensure_capacity(struct git_graph *graph, int num_columns) graph->column_capacity *=2 ; }while (graph->column_capacity < num_columns); - graph->columns =xrealloc (graph->columns, -sizeof (struct column ) * - graph->column_capacity); - graph->new_columns =xrealloc (graph->new_columns, -sizeof (struct column ) * - graph->column_capacity); - graph->mapping =xrealloc (graph->mapping, -sizeof (int ) *2 * graph->column_capacity); - graph->new_mapping =xrealloc (graph->new_mapping, -sizeof (int ) *2 * graph->column_capacity); +REALLOC_ARRAY (graph->columns, graph->column_capacity); +REALLOC_ARRAY (graph->new_columns, graph->column_capacity); +REALLOC_ARRAY (graph->mapping, graph->column_capacity *2 ); +REALLOC_ARRAY (graph->new_mapping, graph->column_capacity *2 ); }/*
khash.h +4 -8
@@ -121,13 +121,9 @@ static const double __ac_HASH_UPPER = 0.77;if (!new_flags)return -1 ; \memset (new_flags,0xaa ,__ac_fsize (new_n_buckets) *sizeof (khint32_t)); \if (h->n_buckets < new_n_buckets) {/* expand */ \ -khkey_t *new_keys = (khkey_t *)xrealloc ((void *)h->keys, new_n_buckets *sizeof (khkey_t)); \ -if (!new_keys)return -1 ; \ - h->keys = new_keys; \ +REALLOC_ARRAY (h->keys, new_n_buckets); \if (kh_is_map) { \ -khval_t *new_vals = (khval_t *)xrealloc ((void *)h->vals, new_n_buckets *sizeof (khval_t)); \ -if (!new_vals)return -1 ; \ - h->vals = new_vals; \ +REALLOC_ARRAY (h->vals, new_n_buckets); \ } \ }/* otherwise shrink */ \ } \
@@ -160,8 +156,8 @@ static const double __ac_HASH_UPPER = 0.77; } \ } \if (h->n_buckets > new_n_buckets) {/* shrink the hash table */ \ - h->keys = (khkey_t *)xrealloc ((void *)h->keys, new_n_buckets *sizeof (khkey_t)); \ -if (kh_is_map) h->vals = (khval_t *)xrealloc ((void *)h->vals, new_n_buckets *sizeof (khval_t)); \ +REALLOC_ARRAY (h->keys, new_n_buckets); \ +if (kh_is_map)REALLOC_ARRAY (h->vals, new_n_buckets); \ } \free (h->flags);/* free the working space */ \ h->flags = new_flags; \
line-log.c +1 -1
@@ -533,7 +533,7 @@ static void fill_line_ends(struct diff_filespec *spec, long *lines, }/* shrink the array to fit the elements */ - ends =xrealloc (ends, cur *sizeof (*ends)); +REALLOC_ARRAY (ends, cur); *lines = cur-1 ; *line_ends = ends; }
object.c +1 -1
@@ -312,7 +312,7 @@ static void add_object_array_with_mode_context(struct object *obj, const char *nif (nr >= alloc) { alloc = (alloc +32 ) *2 ; - objects =xrealloc (objects, alloc *sizeof (*objects)); +REALLOC_ARRAY (objects, alloc); array->alloc = alloc; array->objects = objects; }
pack-bitmap-write.c +1 -2
@@ -111,8 +111,7 @@ static inline void push_bitmapped_commit(struct commit *commit, struct ewah_bitm {if (writer.selected_nr >= writer.selected_alloc) { writer.selected_alloc = (writer.selected_alloc +32 ) *2 ; - writer.selected =xrealloc (writer.selected, - writer.selected_alloc *sizeof (struct bitmapped_commit )); +REALLOC_ARRAY (writer.selected, writer.selected_alloc); } writer.selected[writer.selected_nr].commit = commit;
pack-bitmap.c +2 -4
@@ -400,10 +400,8 @@ static int ext_index_add_object(struct object *object, const char *name)if (hash_ret >0 ) {if (eindex->count >= eindex->alloc) { eindex->alloc = (eindex->alloc +16 ) *3 /2 ; - eindex->objects =xrealloc (eindex->objects, - eindex->alloc *sizeof (struct object *)); - eindex->hashes =xrealloc (eindex->hashes, - eindex->alloc *sizeof (uint32_t )); +REALLOC_ARRAY (eindex->objects, eindex->alloc); +REALLOC_ARRAY (eindex->hashes, eindex->alloc); } bitmap_pos = eindex->count;
pack-objects.c +1 -2
@@ -92,8 +92,7 @@ struct object_entry *packlist_alloc(struct packing_data *pdata,if (pdata->nr_objects >= pdata->nr_alloc) { pdata->nr_alloc = (pdata->nr_alloc +1024 ) *3 /2 ; - pdata->objects =xrealloc (pdata->objects, - pdata->nr_alloc *sizeof (*new_entry)); +REALLOC_ARRAY (pdata->objects, pdata->nr_alloc); } new_entry = pdata->objects + pdata->nr_objects++;
revision.c +1 -1
@@ -1397,7 +1397,7 @@ static void prepare_show_merge(struct rev_info *revs)continue ;if (ce_path_match (ce, &revs->prune_data,NULL )) { prune_num++; - prune =xrealloc (prune,sizeof (*prune) * prune_num); +REALLOC_ARRAY (prune, prune_num); prune[prune_num-2 ] = ce->name; prune[prune_num-1 ] =NULL ; }
sh-i18n--envsubst.c +1 -4
@@ -208,11 +208,8 @@ string_list_append (string_list_ty *slp, const char *s)/* Grow the list. */ if (slp->nitems >= slp->nitems_max) { -size_t nbytes; - slp->nitems_max = slp->nitems_max *2 +4 ; - nbytes = slp->nitems_max *sizeof (slp->item[0 ]); - slp->item = (const char **)xrealloc (slp->item, nbytes); +REALLOC_ARRAY (slp->item, slp->nitems_max); }/* Add the string to the end of the list. */
shallow.c +1 -2
@@ -392,8 +392,7 @@ static uint32_t *paint_alloc(struct paint_info *info)void *p;if (!info->slab_count || info->free + size > info->end) { info->slab_count++; - info->slab =xrealloc (info->slab, - info->slab_count *sizeof (*info->slab)); +REALLOC_ARRAY (info->slab, info->slab_count); info->free =xmalloc (COMMIT_SLAB_SIZE ); info->slab[info->slab_count -1 ] = info->free; info->end = info->free +COMMIT_SLAB_SIZE ;
string-list.c +1 -2
@@ -43,8 +43,7 @@ static int add_entry(int insert_at, struct string_list *list, const char *stringif (list->nr +1 >= list->alloc) { list->alloc +=32 ; - list->items =xrealloc (list->items, list->alloc - *sizeof (struct string_list_item )); +REALLOC_ARRAY (list->items, list->alloc); }if (index < list->nr) memmove(list->items + index + 1, list->items + index,
walker.c +2 -2
@@ -228,8 +228,8 @@ int walker_targets_stdin(char ***target, const char ***write_ref)if (targets >= targets_alloc) { targets_alloc = targets_alloc ? targets_alloc *2 :64 ; - *target =xrealloc (*target, targets_alloc *sizeof (**target)); - *write_ref =xrealloc (*write_ref, targets_alloc *sizeof (**write_ref)); +REALLOC_ARRAY (*target, targets_alloc); +REALLOC_ARRAY (*write_ref, targets_alloc); } (*target)[targets] =xstrdup (tg_one); (*write_ref)[targets] = rf_one ?xstrdup (rf_one) :NULL ;
Signature · 1 line
2.1.0
Re: [PATCH 2/2] use REALLOC_ARRAY for changing the allocation size of arrays
khash.h
Quoted 2 lines · René Scharfe
khash.h +0
-0
(not really about this patch) Where did this file come from? Do we want to be able to sync with upstream to get later bugfixes (e.g.,
https://github.com/attractivechaos/klib/commit/000f0890)? If so, it might make sense to avoid unnecessary changes to the file so it's easier to see when it's safe to replace the file wholesale with a new version.
Curious,
Jonathan
Re: [PATCH 2/2] use REALLOC_ARRAY for changing the allocation size of arrays
Am 24.09.2014 um 20:47 schrieb Jonathan Nieder:
Quoted 11 lines
khash.h
Quoted 2 lines · René Scharfe
khash.h +0
-0(not really about this patch) Where did this file come from? Do we want to be able to sync with upstream to get later bugfixes (e.g.,
https://github.com/attractivechaos/klib/commit/000f0890)? If so, it might make sense to avoid unnecessary changes to the file so it's easier to see when it's safe to replace the file wholesale with a new version.
True. For this patch we're safe, I think, because it already called xrealloc before I touched it (and has been doing that since it was imported into git).
René
Quoted 8 lines · René Scharfe
Am 16.09.2014 um 05:04 schrieb Junio C Hamano:
Quoted 7 lines · René Scharfe
Quoted 1 line
+#define
REALLOCARRAY(x, alloc)x =xrealloc((x), (alloc) * sizeof(*(x)))I have been wondering if "x" could be an expression that has an operator that binds weaker than the assignment '='. That may necessitate the LHS of the assignment to be somehow marked as bound the tightest, i.e.
Quoted 8 lines
Quoted 3 lines
#define
REALLOC_ARRAY(x, alloc)(x) =xrealloc((x), (alloc) * sizeof(*(x)))Or am I being overly silly?
ALLOC_GROWdid well without that. I can't think of a good use case for a complex expression on the right-hand side. That said, I think I still have a spare matching pair of parentheses lying around here somewhere, so let's play it safe and use them. :)
Yeah, as long as (any) is still an lvalue for any lvalue for everybody's compiler, (x) = ... expression ... is safer, but taking cue from ALLOC_GROW(), I would say I was probably being slightly overly silly.