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

Re: [PATCH] range-diff: fix some 'hdr-check' and sparse warnings

From
Jeff King <peff@peff.net>
Date
Jul 16, 2019, 20:01 UTC
Message-ID
<20190716200101.GA6558@sigill.intra.peff.net>
In-Reply-To
<xmqqo91t3itl.fsf@gitster-ct.c.googlers.com>
On Tue, Jul 16, 2019 at 12:01:10PM -0700, Junio C Hamano wrote:
Show 8 quoted lines
> And that "quiet and nice" form is a moral equivalent of
> 
> 	struct foo foo = { 0 };
> 
> that has been discussed in this thread.  I'd rather not to see it
> turned into distinct FOO_INIT, BAR_INIT, etc. to force the reader to
> think these structures all need their specific initialization and
> wonder what's the reason for each of them.

I'm on the fence for that style myself. But we've definitely been trending in that direction. Look at `git grep _INIT *.h`, many of which are clearly zero-initializers.

I do think it's nice to be able to modify the initializers later and feel confident that you're catching all of the users. But even then:

  - it's not like we get any kind of static warning for a
    zero-initialized variant (be it static or with a manual {0}
    initializer)
  - I know I've run into problems where code assumed memset() worked,
    but it didn't (I think diff_options was one such case).
So at best it's "feel more confident", not "feel confident". :)
Show 11 quoted lines
> One universal "struct foo foo = STRUCT_ZERO_INIT;" that is applied
> to all kinds of structure I could live with (but only if we have a
> good way to squelch sparse from bitching about it).  Perhaps we
> could define it as "{}" for GCC, while keeping it "{ 0 }" for
> others.  As I said, { 0 } is undefensible if we insist that a null
> pointer must be spelled NULL and not 0 (as CodingGuidelines says),
> but as long as we declare that we take "{ 0 }" as a mere convention
> (like we used to use the "int foo = foo;" convention to squelch
> "uninitialized but used" warnings) that is outside the purview of
> language-lawyers, I am perfectly fine with it, and if it is hidden
> behind a macro, that would be even better ;-)

Yeah, I am OK with that. My big question is if we use "{}" for gcc (and compatible friends), does that squelch all of the complaints from other compilers and tools that might see the "{0}" version? In particular, does it work for sparse?

-Peff
Previous: Junio C HamanoNext: Junio C Hamano
Message 15 of 23 in “range-diff: fix some 'hdr-check' and sparse warnings”
  1. range-diff: fix some 'hdr-check' and sparse warningsRamsay Jones, Jul 11, 2019
  2. Johannes SixtJul 12, 2019
  3. Junio C HamanoJul 12, 2019
  4. Johannes SixtJul 13, 2019
  5. Johannes SixtJul 13, 2019
  6. Carlo ArenasJul 13, 2019
  7. Junio C HamanoJul 13, 2019
  8. Carlo ArenasJul 13, 2019
  9. Jeff KingJul 14, 2019
  10. Johannes SixtJul 14, 2019
  11. Jeff KingJul 15, 2019
  12. Johannes SixtJul 15, 2019
  13. Jeff KingJul 15, 2019
  14. Junio C HamanoJul 16, 2019
  15. Jeff KingJul 16, 2019
  16. Junio C HamanoJul 17, 2019
  17. Jeff KingJul 17, 2019
  18. Junio C HamanoJul 17, 2019
  19. Johannes SixtJul 17, 2019
  20. Johannes SchindelinJul 15, 2019
  21. Junio C HamanoOct 2, 2020
  22. Johannes SchindelinOct 4, 2020
  23. Johannes SixtJul 14, 2019

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.