All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Martin Ågren" <martin.agren@gmail.com>
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Git Mailing List" <git@vger.kernel.org>,
	"Jeff King" <peff@peff.net>
Subject: Re: [PATCH 1/5] *.h: move some *_INIT to designated initializers
Date: Thu, 01 Jul 2021 08:56:46 -0700	[thread overview]
Message-ID: <xmqq8s2qrps1.fsf@gitster.g> (raw)
In-Reply-To: <CAN0heSpsg=bwX9gbYfeWYqUbuen-zArR69FWxNmdtcQW_RubzA@mail.gmail.com> ("Martin Ågren"'s message of "Thu, 1 Jul 2021 17:00:02 +0200")

Martin Ågren <martin.agren@gmail.com> writes:

>> -#define STRING_LIST_INIT_NODUP { NULL, 0, 0, 0, NULL }
>> +#define STRING_LIST_INIT_NODUP { 0 }
>
> This "NODUP" one is a bit of an odd case though. You don't actually
> change it to use designated initializers, as the patch says. Instead you
> change it in a slightly different way. In a sense, you assign the
> literal zero not to strdup_strings, but to the first field, which is a
> pointer, where I would have expected NULL.
>
> I think there's been some recent-ish "we can assign `{ 0 }` even if the
> first field is a pointer, because it's idiomatic and works out fine"
> even if assigning 0 to a pointer looks a bit odd. So it might not be
> wrong as such, but it doesn't match what the commit message claims, and
> I think it would be more clear to use `{ .strdup_strings = 0 }` here:
> We're explicitly interested in *not* duplicating the strings. It's not
> just some default "let's leave everything as zero/NULL".

Well reasoned.  I agree that it is not wrong per-se, and the
solution to use designated initializer is indeed in line with the
theme of the topic.


  reply	other threads:[~2021-07-01 15:56 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-01 10:51 [PATCH 0/5] *.[ch]: don't duplicate *_init() and *_INIT logic Ævar Arnfjörð Bjarmason
2021-07-01 10:51 ` [PATCH 1/5] *.h: move some *_INIT to designated initializers Ævar Arnfjörð Bjarmason
2021-07-01 15:00   ` Martin Ågren
2021-07-01 15:56     ` Junio C Hamano [this message]
2021-07-01 16:08     ` Ævar Arnfjörð Bjarmason
2021-07-01 10:51 ` [PATCH 2/5] *.c *_init(): define in terms of corresponding *_INIT macro Ævar Arnfjörð Bjarmason
2021-07-01 10:51 ` [PATCH 3/5] dir.[ch]: replace dir_init() with DIR_INIT Ævar Arnfjörð Bjarmason
2021-07-01 10:51 ` [PATCH 4/5] string-list.[ch]: add a string_list_init_{nodup,dup}() Ævar Arnfjörð Bjarmason
2021-07-01 10:51 ` [PATCH 5/5] string-list.h users: change to use *_{nodup,dup}() Ævar Arnfjörð Bjarmason
2021-07-01 16:13 ` [PATCH 0/5] *.[ch]: don't duplicate *_init() and *_INIT logic Jeff King

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=xmqq8s2qrps1.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 \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.