Re: [PATCH 07/18] convert trivial cases to FLEX_ARRAY macros

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 16, 2016, 02:17 UTC
Message-ID: <CAPig+cSQG7gWStpRy76D_YzuCFPsXJLBzXCjQ-X_Q3sZthx3iw@mail.gmail.com>
In-Reply-To: <20160215215244.GG10287@sigill.intra.peff.net>

On Mon, Feb 15, 2016 at 4:52 PM, Jeff King <peff@peff.net> wrote:

Show 15 quoted lines

> Using FLEX_ARRAY macros reduces the amount of manual
> computation size we have to do. It also ensures we don't
> overflow size_t, and it makes sure we write the same number
> of bytes that we allocated.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> diff --git a/builtin/reflog.c b/builtin/reflog.c
> @@ -412,8 +410,7 @@ static struct reflog_expire_cfg *find_cfg_ent(const char *pattern, size_t len)
>                     !memcmp(ent->pattern, pattern, len))
>                         return ent;
>
> -       ent = xcalloc(1, (sizeof(*ent) + len));
> -       memcpy(ent->pattern, pattern, len);
> +       FLEX_ALLOC_MEM(ent, pattern, pattern, len);

Does the incoming 'len' already account for the NUL terminator, or was the original code underallocating?

Answering my own question: Looking at reflog_expire_config() and parse_config_key(), I gather that 'len' already accounts for the NUL, thus the new code is overallocating (which should not be a problem).

Show 10 quoted lines

>         ent->len = len;
>         *reflog_expire_cfg_tail = ent;
>         reflog_expire_cfg_tail = &(ent->next);
> diff --git a/hashmap.c b/hashmap.c
> @@ -256,10 +256,9 @@ const void *memintern(const void *data, size_t len)
>         e = hashmap_get(&map, &key, data);
>         if (!e) {
>                 /* not found: create it */
> -               e = xmallocz(sizeof(struct pool_entry) + len);
> +               FLEX_ALLOC_MEM(e, data, data, len);

Ditto. I guess the new code is overallocating (which should be okay).

Show 6 quoted lines

>                 hashmap_entry_init(e, key.ent.hash);
>                 e->len = len;
> -               memcpy(e->data, data, len);
>                 hashmap_add(&map, e);
>         }
>         return e->data;