threads / patch / 58197

patchstring_list: use the right kind of STRING_LIST_INIT

Subject: [PATCH] string_list: use the right kind of STRING_LIST_INIT

## tl;dr

5 messages between Jul 21, 2022 and Jul 25, 2022. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Junio C Hamano· Jul 21, 2022, 01:02 UTC · lore

Since 4a4b4cda (builtin-remote: Make "remote -v" display push urls, 2009-06-13), the string_list that was initialized with 0 in its strdup_string member is immediately made to strdup its key strings by flipping the strdup_string member to true. When 183113a5 (string_list: Add STRING_LIST_INIT macro and make use of it., 2010-07-04) has introduced STRING_LIST_INIT macros, it mechanically replaced the initialization to STRING_LIST_INIT_NODUP.

Instead, just use the other initialization macro to make it strdup the key from the beginning.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/remote.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Show changes to diff +1 −2

diff --git c/builtin/remote.c w/builtin/remote.c
index d9b8746cb3..c713463d89 100644
--- c/builtin/remote.c
+++ w/builtin/remote.c
@@ -1229,10 +1229,9 @@ static int get_one_entry(struct remote *remote, void *priv)
 
 static int show_all(void)
 {
-	struct string_list list = STRING_LIST_INIT_NODUP;
+	struct string_list list = STRING_LIST_INIT_DUP;
 	int result;
 
-	list.strdup_strings = 1;
 	result = for_each_remote(get_one_entry, &list);
 
 	if (!result) {

Elijah Newren· Jul 21, 2022, 03:28 UTC · re: Junio C Hamano · lore

Re: [PATCH] string_list: use the right kind of STRING_LIST_INIT

On Wed, Jul 20, 2022 at 6:19 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 33 quoted lines

>
> Since 4a4b4cda (builtin-remote: Make "remote -v" display push urls,
> 2009-06-13), the string_list that was initialized with 0 in its
> strdup_string member is immediately made to strdup its key strings
> by flipping the strdup_string member to true.  When 183113a5
> (string_list: Add STRING_LIST_INIT macro and make use of it.,
> 2010-07-04) has introduced STRING_LIST_INIT macros, it mechanically
> replaced the initialization to STRING_LIST_INIT_NODUP.
>
> Instead, just use the other initialization macro to make it strdup
> the key from the beginning.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  builtin/remote.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git c/builtin/remote.c w/builtin/remote.c
> index d9b8746cb3..c713463d89 100644
> --- c/builtin/remote.c
> +++ w/builtin/remote.c
> @@ -1229,10 +1229,9 @@ static int get_one_entry(struct remote *remote, void *priv)
>
>  static int show_all(void)
>  {
> -       struct string_list list = STRING_LIST_INIT_NODUP;
> +       struct string_list list = STRING_LIST_INIT_DUP;
>         int result;
>
> -       list.strdup_strings = 1;
>         result = for_each_remote(get_one_entry, &list);
>
>         if (!result) {

Makes sense, but shouldn't the subject have s/string-list:/remote:/ ?

Junio C Hamano· Jul 21, 2022, 04:45 UTC · re: Elijah Newren · lore

Re: [PATCH] string_list: use the right kind of STRING_LIST_INIT

Elijah Newren <newren@gmail.com> writes:

> Makes sense, but shouldn't the subject have s/string-list:/remote:/ ?

Oh, you are absolutely right.

I was influenced by 183113a5 (string_list: Add STRING_LIST_INIT macro and make use of it., 2010-07-04), which I was staring at too much while thinking that this fix was finishing up what it started.

Will amend.  Thanks.

Ævar Arnfjörð Bjarmason· Jul 21, 2022, 05:55 UTC · re: Junio C Hamano · lore

Re: [PATCH] string_list: use the right kind of STRING_LIST_INIT

On Wed, Jul 20 2022, Junio C Hamano wrote:

Show 32 quoted lines

> Since 4a4b4cda (builtin-remote: Make "remote -v" display push urls,
> 2009-06-13), the string_list that was initialized with 0 in its
> strdup_string member is immediately made to strdup its key strings
> by flipping the strdup_string member to true.  When 183113a5
> (string_list: Add STRING_LIST_INIT macro and make use of it.,
> 2010-07-04) has introduced STRING_LIST_INIT macros, it mechanically
> replaced the initialization to STRING_LIST_INIT_NODUP.
>
> Instead, just use the other initialization macro to make it strdup
> the key from the beginning.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  builtin/remote.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git c/builtin/remote.c w/builtin/remote.c
> index d9b8746cb3..c713463d89 100644
> --- c/builtin/remote.c
> +++ w/builtin/remote.c
> @@ -1229,10 +1229,9 @@ static int get_one_entry(struct remote *remote, void *priv)
>  
>  static int show_all(void)
>  {
> -	struct string_list list = STRING_LIST_INIT_NODUP;
> +	struct string_list list = STRING_LIST_INIT_DUP;
>  	int result;
>  
> -	list.strdup_strings = 1;
>  	result = for_each_remote(get_one_entry, &list);
>  
>  	if (!result) {

Heh :) I think it's safe to give this my Reviewed-By, since I've been running with this patch for almost a year now: https://github.com/avar/git/commit/bdf4466eb92

It's on a branch of some WIP string_list API fixes that I haven't sent in, but the fix looks good to me...

Junio C Hamano· Jul 25, 2022, 19:01 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH] string_list: use the right kind of STRING_LIST_INIT

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 6 quoted lines

> Heh :) I think it's safe to give this my Reviewed-By, since I've been
> running with this patch for almost a year now:
> https://github.com/avar/git/commit/bdf4466eb92
>
> It's on a branch of some WIP string_list API fixes that I haven't sent
> in, but the fix looks good to me...

I do not think it is all that urgent, but I do not think it is worth wasting our braincells to remember that a WIP series is waiting to touch and update this area again, so I'll merge this single patch down to 'next' and 'master' soonish.

Thanks.