git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
From: Johannes Sixt <j6t@kdbg.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: Ramsay Jones <ramsay@ramsayjones.plus.com>,
	Thomas Gummerer <t.gummerer@gmail.com>,
	GIT Mailing-list <git@vger.kernel.org>, Jeff King <peff@peff.net>
Subject: Re: [PATCH] range-diff: fix some 'hdr-check' and sparse warnings
Date: Sun, 14 Jul 2019 10:15:26 +0200
Message-ID: <9d6c7595-e320-7fa2-1f02-2f078b3831fa@kdbg.org> (raw)
In-Reply-To: <xmqq1rytpqse.fsf@gitster-ct.c.googlers.com>

Am 13.07.19 um 23:29 schrieb Junio C Hamano:
> I do not think this position is maintainable, especially if you
> agree with me (and everybody else, including sparse) that this is a
> bad idea:
> 
>>   struct string_list dup_it = { 0, 0, 0, 1, 0 };
> 
> The way I read "6.7.8 Initialization" (sorry, I only have committee
> draft wg14/n1124 of 2005 handy) is that
> 
> 	struct patch patch = { 0 };
> 
> has an initializer for a structure with an automatic storage
> duration, and for each of the subsequent fields of the structure
> (i.e. we ran out the initializer after dealing with that single zero
> that talks about the first field), due to "all subobjects that are
> not initialized explicitly shall be initialized implicitly the same
> as objects that have static storage duration." rule, "if it has a
> pointer type, it is initialized to a null pointer", which is exactly
> in line with your (and our) position that the first example I left
> in the above (new_name gets assigned NULL).  So we are fine with the
> fields that are not speled out.
> 
> But then what about the explicitly spelled out 0 for the first
> field?

You are putting too much meaning in the token '0' when it appears in the
token sequence '= { 0 }'. I understand this sequence as a signal to the
reader of the code that "the whole struct is to be zero-initialized".
NOT "the first member is set to zero and everything else the default
zero value".

It just so happens that the compiler does the right thing with that '0'
regardless of what type the first member has. (It even works when it is
a struct, Peff!) That zero is a null pointer constant if the first
member happens to be a pointer, i.e., the same value that is used for
all other implicitly zero-initialized pointers.

>  It is like an assignment, so by arguing that we should have
> 0 over there and not NULL, you are essentially arguing for
> 
> 	patch.new_name = 0; /* not NULL */
> 
> aren't you?

No,no. You are stretching my argument too far. I really only mean the
sequence = { 0 } as a signal. When this form of zero-initialization
becomes established, it takes away mental burden from the reader. It
need not be decomposed into its parts; there is no question of what is
it that is initialized by '0', and what happens to the rest of the
struct. It means "zero, all of it" without thinking.

> I wish if we could say
> 
> 	struct patch patch = {};
> 
> so that we an leave all fields to have their natural zero value

YES!

-- Hannes

      parent reply index

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-11 22:03 Ramsay Jones
2019-07-12  5:21 ` Johannes Sixt
2019-07-12 16:44   ` Junio C Hamano
2019-07-13 10:44     ` Johannes Sixt
2019-07-13 12:18       ` Johannes Sixt
2019-07-13 12:56       ` Carlo Arenas
2019-07-13 21:29       ` Junio C Hamano
2019-07-13 22:22         ` Carlo Arenas
2019-07-14  0:51           ` Jeff King
2019-07-14  8:30             ` Johannes Sixt
2019-07-15 14:46               ` Jeff King
2019-07-15 17:30                 ` Johannes Sixt
2019-07-15 18:15                   ` Jeff King
2019-07-16 19:01                     ` Junio C Hamano
2019-07-16 20:01                       ` Jeff King
2019-07-17 18:13                         ` Junio C Hamano
2019-07-17 19:21                           ` Jeff King
2019-07-17 20:10                             ` Junio C Hamano
2019-07-17 17:23                       ` Johannes Sixt
2019-07-15 14:47           ` Johannes Schindelin
2019-07-14  8:15         ` Johannes Sixt [this message]

Reply instructions:

You may reply publically 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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9d6c7595-e320-7fa2-1f02-2f078b3831fa@kdbg.org \
    --to=j6t@kdbg.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=ramsay@ramsayjones.plus.com \
    --cc=t.gummerer@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

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror http://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox