From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, "Jeff King" <peff@peff.net>,
"Martin Ågren" <martin.agren@gmail.com>,
"Phillip Wood" <phillip.wood123@gmail.com>
Subject: Re: [PATCH v2 4/5] builtin/remote.c: add and use a REF_STATES_INIT
Date: Mon, 27 Sep 2021 16:56:44 -0700 [thread overview]
Message-ID: <xmqqv92lmv2b.fsf@gitster.g> (raw)
In-Reply-To: <87v92lzihd.fsf@evledraar.gmail.com> (=?utf-8?B?IsOGdmFyIEFy?= =?utf-8?B?bmZqw7Zyw7A=?= Bjarmason"'s message of "Tue, 28 Sep 2021 01:38:16 +0200")
Ævar Arnfjörð Bjarmason <avarab@gmail.com> 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.
>>> + 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.
next prev parent reply other threads:[~2021-09-27 23:56 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-27 0:53 [PATCH 0/6] Non-trivial designated initializer conversion Ævar Arnfjörð Bjarmason
2021-09-27 0:53 ` [PATCH 1/6] daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macro Ævar Arnfjörð Bjarmason
2021-09-27 0:53 ` [PATCH 2/6] builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macro Ævar Arnfjörð Bjarmason
2021-09-27 0:53 ` [PATCH 3/6] shortlog: use designated initializer for "struct shortlog" Ævar Arnfjörð Bjarmason
2021-09-27 9:06 ` Phillip Wood
2021-09-27 10:52 ` Ævar Arnfjörð Bjarmason
2021-09-27 0:53 ` [PATCH 4/6] urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT Ævar Arnfjörð Bjarmason
2021-09-27 0:53 ` [PATCH 5/6] builtin/remote.c: add and use a REF_STATES_INIT Ævar Arnfjörð Bjarmason
2021-09-27 0:53 ` [PATCH 6/6] builtin/remote.c: add and use SHOW_INFO_INIT Ævar Arnfjörð Bjarmason
2021-09-27 12:58 ` [PATCH v2 0/5] Non-trivial designated initializer conversion Ævar Arnfjörð Bjarmason
2021-09-27 12:58 ` [PATCH v2 1/5] daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macro Ævar Arnfjörð Bjarmason
2021-09-27 12:58 ` [PATCH v2 2/5] builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macro Ævar Arnfjörð Bjarmason
2021-09-27 12:58 ` [PATCH v2 3/5] urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT Ævar Arnfjörð Bjarmason
2021-09-27 22:12 ` Junio C Hamano
2021-09-27 12:58 ` [PATCH v2 4/5] builtin/remote.c: add and use a REF_STATES_INIT Ævar Arnfjörð Bjarmason
2021-09-27 23:04 ` Junio C Hamano
2021-09-27 23:38 ` Ævar Arnfjörð Bjarmason
2021-09-27 23:56 ` Junio C Hamano [this message]
2021-09-27 12:58 ` [PATCH v2 5/5] builtin/remote.c: add and use SHOW_INFO_INIT Ævar Arnfjörð Bjarmason
2021-10-01 10:27 ` [PATCH v3 0/6] Non-trivial designated initializer conversion Ævar Arnfjörð Bjarmason
2021-10-01 10:27 ` [PATCH v3 1/6] daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macro Ævar Arnfjörð Bjarmason
2021-10-01 10:27 ` [PATCH v3 2/6] builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macro Ævar Arnfjörð Bjarmason
2021-10-01 10:27 ` [PATCH v3 3/6] urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT Ævar Arnfjörð Bjarmason
2021-10-01 10:27 ` [PATCH v3 4/6] builtin/remote.c: add and use a REF_STATES_INIT Ævar Arnfjörð Bjarmason
2021-10-01 10:27 ` [PATCH v3 5/6] builtin/remote.c: add and use SHOW_INFO_INIT Ævar Arnfjörð Bjarmason
2021-10-01 10:27 ` [PATCH v3 6/6] unpack-trees.[ch]: define and use a UNPACK_TREES_OPTIONS_INIT Ævar Arnfjörð Bjarmason
2021-10-01 21:39 ` Junio C Hamano
2021-10-02 20:16 ` [PATCH v4 0/5] Non-trivial designated initializer conversion Ævar Arnfjörð Bjarmason
2021-10-02 20:16 ` [PATCH v4 1/5] daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macro Ævar Arnfjörð Bjarmason
2021-10-02 20:16 ` [PATCH v4 2/5] builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macro Ævar Arnfjörð Bjarmason
2021-10-02 20:16 ` [PATCH v4 3/5] urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT Ævar Arnfjörð Bjarmason
2021-10-02 20:16 ` [PATCH v4 4/5] builtin/remote.c: add and use a REF_STATES_INIT Ævar Arnfjörð Bjarmason
2021-10-02 20:16 ` [PATCH v4 5/5] builtin/remote.c: add and use SHOW_INFO_INIT Ævar Arnfjörð Bjarmason
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqv92lmv2b.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=martin.agren@gmail.com \
--cc=peff@peff.net \
--cc=phillip.wood123@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).