Re: [PATCH v2 4/5] builtin/remote.c: add and use a REF_STATES_INIT
From: Junio C Hamano <hidden>
Date: 2021-09-27 23:04:45
Ævar Arnfjörð Bjarmason [off-list ref] writes:
quoted hunk
Use a new REF_STATES_INIT designated initializer instead of assigning to the "strdup_strings" member of the previously memzero()'d version of this struct. The pattern of assigning to "strdup_strings" dates back to 211c89682ee (Make git-remote a builtin, 2008-02-29) (when it was "strdup_paths"), i.e. long before we used anything like our current established *_INIT patterns consistently. Then in e61e0cc6b70 (builtin-remote: teach show to display remote HEAD, 2009-02-25) and e5dcbfd9ab7 (builtin-remote: new show output style for push refspecs, 2009-02-25) we added some more of these. As it turns out we only initialized this struct three times, all the other uses were of pointers to those initialized structs. So let's initialize it in those three places, skip the memset(), and pass those structs down appropriately. Signed-off-by: Ævar Arnfjörð Bjarmason <redacted> --- builtin/remote.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)diff --git a/builtin/remote.c b/builtin/remote.c index 7f88e6ce9de..160dd954f74 100644 --- a/builtin/remote.c +++ b/builtin/remote.c@@ -344,6 +344,14 @@ struct ref_states { int queried; }; +#define REF_STATES_INIT { \ + .new_refs = STRING_LIST_INIT_DUP, \ + .stale = STRING_LIST_INIT_DUP, \ + .tracked = STRING_LIST_INIT_DUP, \ + .heads = STRING_LIST_INIT_DUP, \ + .push = STRING_LIST_INIT_DUP, \ +}
So, now everybody owns the string, but ...
quoted hunk
static int get_ref_states(const struct ref *remote_refs, struct ref_states *states) { struct ref *fetch_map = NULL, **tail = &fetch_map;@@ -355,9 +363,6 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat die(_("Could not get fetch map for refspec %s"), states->remote->fetch.raw[i]); - states->new_refs.strdup_strings = 1; - states->tracked.strdup_strings = 1; - states->stale.strdup_strings = 1;
... we used to set up selectively to own. How would we make sure after this change we are not adding leaks? Is there a way to do so mechanically?
- struct ref_states states;
+ struct ref_states states = REF_STATES_INIT;
struct string_list refs_to_prune = STRING_LIST_INIT_NODUP;
struct string_list_item *item;
const char *dangling_msg = dry_run
? _(" %s will become dangling!")
: _(" %s has become dangling!");
- memset(&states, 0, sizeof(states));
get_remote_ref_states(remote, &states, GET_REF_STATES);Like this one, get_remote_ref_states() used to receive states that are set to borrow strings, but now we get duplicated strings, right? Are we leaking whatever strings we push to these string lists now?