git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 4/5] builtin/remote.c: add and use a REF_STATES_INIT

From
Junio C Hamano <gitster@pobox.com>
Date
Sep 27, 2021, 23:56 UTC
Message-ID
<xmqqv92lmv2b.fsf@gitster.g>
In-Reply-To
<87v92lzihd.fsf@evledraar.gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 11 quoted lines
> 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.

Show 20 quoted lines
>>> +	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.
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 18 of 33 in “Non-trivial designated initializer conversion”
  1. 0/6 Non-trivial designated initializer conversionÆvar Arnfjörð Bjarmason, Sep 27, 2021
  2. 1/6 daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macroÆvar Arnfjörð Bjarmason, Sep 27, 2021
  3. 2/6 builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macroÆvar Arnfjörð Bjarmason, Sep 27, 2021
  4. 3/6 shortlog: use designated initializer for "struct shortlog"Ævar Arnfjörð Bjarmason, Sep 27, 2021
  5. Phillip WoodSep 27, 2021
  6. Ævar Arnfjörð BjarmasonSep 27, 2021
  7. 4/6 urlmatch.[ch]: add and use URLMATCH_CONFIG_INITÆvar Arnfjörð Bjarmason, Sep 27, 2021
  8. 5/6 builtin/remote.c: add and use a REF_STATES_INITÆvar Arnfjörð Bjarmason, Sep 27, 2021
  9. 6/6 builtin/remote.c: add and use SHOW_INFO_INITÆvar Arnfjörð Bjarmason, Sep 27, 2021
  10. 0/5 Non-trivial designated initializer conversionÆvar Arnfjörð Bjarmason, Sep 27, 2021
  11. 1/5 daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macroÆvar Arnfjörð Bjarmason, Sep 27, 2021
  12. 2/5 builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macroÆvar Arnfjörð Bjarmason, Sep 27, 2021
  13. 3/5 urlmatch.[ch]: add and use URLMATCH_CONFIG_INITÆvar Arnfjörð Bjarmason, Sep 27, 2021
  14. Junio C HamanoSep 27, 2021
  15. 4/5 builtin/remote.c: add and use a REF_STATES_INITÆvar Arnfjörð Bjarmason, Sep 27, 2021
  16. Junio C HamanoSep 27, 2021
  17. Ævar Arnfjörð BjarmasonSep 27, 2021
  18. Junio C HamanoSep 27, 2021
  19. 5/5 builtin/remote.c: add and use SHOW_INFO_INITÆvar Arnfjörð Bjarmason, Sep 27, 2021
  20. 0/6 Non-trivial designated initializer conversionÆvar Arnfjörð Bjarmason, Oct 1, 2021
  21. 1/6 daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macroÆvar Arnfjörð Bjarmason, Oct 1, 2021
  22. 2/6 builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macroÆvar Arnfjörð Bjarmason, Oct 1, 2021
  23. 3/6 urlmatch.[ch]: add and use URLMATCH_CONFIG_INITÆvar Arnfjörð Bjarmason, Oct 1, 2021
  24. 4/6 builtin/remote.c: add and use a REF_STATES_INITÆvar Arnfjörð Bjarmason, Oct 1, 2021
  25. 5/6 builtin/remote.c: add and use SHOW_INFO_INITÆvar Arnfjörð Bjarmason, Oct 1, 2021
  26. 6/6 unpack-trees.[ch]: define and use a UNPACK_TREES_OPTIONS_INITÆvar Arnfjörð Bjarmason, Oct 1, 2021
  27. Junio C HamanoOct 1, 2021
  28. 0/5 Non-trivial designated initializer conversionÆvar Arnfjörð Bjarmason, Oct 2, 2021
  29. 2/5 builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macroÆvar Arnfjörð Bjarmason, Oct 2, 2021
  30. 1/5 daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macroÆvar Arnfjörð Bjarmason, Oct 2, 2021
  31. 4/5 builtin/remote.c: add and use a REF_STATES_INITÆvar Arnfjörð Bjarmason, Oct 2, 2021
  32. 5/5 builtin/remote.c: add and use SHOW_INFO_INITÆvar Arnfjörð Bjarmason, Oct 2, 2021
  33. 3/5 urlmatch.[ch]: add and use URLMATCH_CONFIG_INITÆvar Arnfjörð Bjarmason, Oct 2, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.