On Fri, May 31, 2019 at 5:40 PM Matthew DeVore <matvore@xxxxxxxxxx> wrote:
>
> Introduce a new macro ALLOC_GROW_BY which automatically zeros the added
> array elements and takes care of updating the nr value. Use the macro in
> code introduced earlier in this patchset.
>
> Signed-off-by: Matthew DeVore <matvore@xxxxxxxxxx>
> ---
> cache.h | 22 ++++++++++++++++++++++
> list-objects-filter-options.c | 17 +++++++----------
> 2 files changed, 29 insertions(+), 10 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index fa8ede9a2d..847fbdeff0 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -652,33 +652,55 @@ int init_db(const char *git_dir, const char *real_git_dir,
> void sanitize_stdfds(void);
> int daemonize(void);
>
> #define alloc_nr(x) (((x)+16)*3/2)
>
> /*
> * Realloc the buffer pointed at by variable 'x' so that it can hold
> * at least 'nr' entries; the number of entries currently allocated
> * is 'alloc', using the standard growing factor alloc_nr() macro.
> *
> + * Consider using ALLOC_GROW_BY instead of ALLOC_GROW as it has some
> + * added niceties.
> + *
> * DO NOT USE any expression with side-effect for 'x', 'nr', or 'alloc'.
> */
> #define ALLOC_GROW(x, nr, alloc) \
> do { \
> if ((nr) > alloc) { \
> if (alloc_nr(alloc) < (nr)) \
> alloc = (nr); \
> else \
> alloc = alloc_nr(alloc); \
> REALLOC_ARRAY(x, alloc); \
> } \
> } while (0)
>
> +/*
> + * Similar to ALLOC_GROW but handles updating of the nr value and
> + * zeroing the bytes of the newly-grown array elements.
> + *
> + * DO NOT USE any expression with side-effect for any of the
> + * arguments.
> + */
Since ALLOC_GROW already doesn't handle this safely, there isn't
necessarily a reason to fix it, but you could read the macro values
into temporary variables inside the do { } while(0) loop in order to
avoid the multiple-expansion side effect issues...
Thanks,
Jake
> +#define ALLOC_GROW_BY(x, nr, increase, alloc) \
> + do { \
> + if (increase) { \
> + size_t new_nr = nr + (increase); \
> + if (new_nr < nr) \
> + BUG("negative growth in ALLOC_GROW_BY"); \
> + ALLOC_GROW(x, new_nr, alloc); \
> + memset((x) + nr, 0, sizeof(*(x)) * (increase)); \
> + nr = new_nr; \
> + } \
> + } while (0)
> +
> /* Initialize and use the cache information */
> struct lock_file;
> void preload_index(struct index_state *index,
> const struct pathspec *pathspec,
> unsigned int refresh_flags);
> int do_read_index(struct index_state *istate, const char *path,
> int must_exist); /* for testting only! */
> int read_index_from(struct index_state *, const char *path,
> const char *gitdir);
> int is_index_unborn(struct index_state *);
> diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
> index 5e98e4a309..d8abe6cfcf 100644
> --- a/list-objects-filter-options.c
> +++ b/list-objects-filter-options.c
> @@ -142,26 +142,24 @@ static int has_reserved_character(
> }
>
> return 0;
> }
>
> static int parse_combine_subfilter(
> struct list_objects_filter_options *filter_options,
> struct strbuf *subspec,
> struct strbuf *errbuf)
> {
> - size_t new_index = filter_options->sub_nr++;
> + size_t new_index = filter_options->sub_nr;
>
> - ALLOC_GROW(filter_options->sub, filter_options->sub_nr,
> - filter_options->sub_alloc);
> - memset(&filter_options->sub[new_index], 0,
> - sizeof(*filter_options->sub));
> + ALLOC_GROW_BY(filter_options->sub, filter_options->sub_nr, 1,
> + filter_options->sub_alloc);
>
> return has_reserved_character(subspec, errbuf) ||
> url_decode(subspec, errbuf) ||
> gently_parse_list_objects_filter(
> &filter_options->sub[new_index], subspec->buf, errbuf);
> }
>
> static int parse_combine_filter(
> struct list_objects_filter_options *filter_options,
> const char *arg,
> @@ -273,27 +271,26 @@ int parse_list_objects_filter(
> /*
> * Make filter_options an LOFC_COMBINE spec so we can trivially
> * add subspecs to it.
> */
> transform_to_combine_type(filter_options);
>
> strbuf_addstr(&filter_options->filter_spec, "+");
> add_url_encoded(&filter_options->filter_spec, arg);
> trace_printf("Generated composite filter-spec: %s\n",
> filter_options->filter_spec.buf);
> - ALLOC_GROW(filter_options->sub, filter_options->sub_nr + 1,
> - filter_options->sub_alloc);
> - filter_options = &filter_options->sub[filter_options->sub_nr++];
> - memset(filter_options, 0, sizeof(*filter_options));
> + ALLOC_GROW_BY(filter_options->sub, filter_options->sub_nr, 1,
> + filter_options->sub_alloc);
>
> parse_error = gently_parse_list_objects_filter(
> - filter_options, arg, &errbuf);
> + &filter_options->sub[filter_options->sub_nr - 1], arg,
> + &errbuf);
> }
> if (parse_error)
> die("%s", errbuf.buf);
> return 0;
> }
>
> int opt_parse_list_objects_filter(const struct option *opt,
> const char *arg, int unset)
> {
> struct list_objects_filter_options *filter_options = opt->value;
> --
> 2.17.1
>
