Thread (1 message) 1 message, 1 author, 2021-09-27

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help