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:56:50
Ævar Arnfjörð Bjarmason [off-list ref] writes:
The ownership of the "states" struct or its lifetime isn't different
after this change. It's only that we're doing:
struct foo = FOO_INIT;
/* use &foo */
Instead of:
struct foo;
memset(&foo, 0, sizeof(foo));
foo->some_list.strdup_strings = 1;No, I am not talking about ownership of "foo" itself. What changes is ownership of some of the string_list embedded in "foo" that used NOT to have strdup_strings member set now have the bit set, so the string list owns the string.
quoted
quoted
+ 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?Ah, yes it *could* happen as a side-effect of this sotr of change that that memset() was implicitly flipping some string_list structs to the equivalent of strdup_strings=0. But that's not the case here, it was just memset() boilerplate, then in get_remote_ref_states() we'd set all the string lists we'd use to "dup".
OK, get_remotes_ref_states() marked all string list to the dup variant, then we won't have a problem. I was trying to make sure that was something you looked at when preparing these patches. Thanks.