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

Re: [PATCH] unpack-objects: fix compilation warning/error due to missing braces

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Jul 12, 2022, 06:41 UTC
Message-ID
<220712.86lesy6cri.gmgdl@evledraar.gmail.com>
In-Reply-To
<CAPig+cQMJcUc4gpRDpR=Q8M44rTjUA7SWgXNmzrnDH7V12z0dQ@mail.gmail.com>
On Tue, Jul 12 2022, Eric Sunshine wrote:
Show 30 quoted lines
> On Mon, Jul 11, 2022 at 12:38 AM Junio C Hamano <gitster@pobox.com> wrote:
>> Eric Sunshine <sunshine@sunshineco.com> writes:
>> > On Sun, Jul 10, 2022 at 10:00 PM Han Xin <chiyutianyi@gmail.com> wrote:
>> >> On Sun, Jul 10, 2022 at 4:12 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
>> >> > [1]: `cc --version` => "Apple LLVM version 10.0.0 (clang-1000.10.44.4)"
>> >> > -       git_zstream zstream = { 0 };
>> >> > +       git_zstream zstream = {{ 0 }};
>> >>
>> >> Not a comment, just wondering, when should I use "{ { 0 } }" and when
>> >> should I use "{ 0 }"?
>> >
>> > I don't have a good answer. More modern `clang` versions don't seem to
>> > complain about plain old `{0}` here, but the older `clang` with which
>> > I'm stuck does complain.
>>
>> I think, from the language-lawyer perspective, "{ 0 }" is how we
>> should spell these initialization when we are not using designated
>> initializers, even when the first member of the struct happens to be
>> a struct.
>>
>> The older clang that complains at you is simply buggy, and I think
>> we had the same issue with older sparse.
>
> I can't tell from your response whether or not you intend to pick up
> this patch. I don't disagree that older clang may be considered buggy
> in this regard, but older clang versions still exist in the wild, and
> we already support them by applying `{{0}}` when appropriate:
>
>     % git grep -n '{ *{ *0 *} *}'
>     builtin/merge-file.c:31: xmparam_t xmp = {{0}};

Not so fast :) If you check out "next", does compiling builtin/merge-file.o there complain on that clang version now? I changed this to the "{ 0 }" form.

If not I wonder if this has to do with one of git_zstream being typedef'd, or with the first member being an embedded struct (I couldn't find another example of that). For the former does the patch at the end & "make builtin/unpack-objects.o" make it go away?

>     builtin/worktree.c:262: struct config_set cs = { { 0 } };
>     oidset.h:25:#define OIDSET_INIT { { 0 } }
>     worktree.c:840: struct config_set cs = { { 0 } };

Uh, and here are some other examples, so those also warn if you make them just a "{ 0 }"?

> so the change made by this patch is in line with existing practice on
> this project.

It is nice though to be able to use standard C99 consistently, where a "{ 0 }" recursively initializes the members, I think that's what your clang version is doing, it's just complaining about it.

Since this is only a warning, and only a practical issue with -Werror I wonder if a config.mak.dev change wouldn't be better, i.e. to provide a -Wno-missing-braces for this older clang version.

The ad-hoc test patch referred to above:
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 43789b8ef29..f08092cb26d 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -110,7 +110,7 @@ static void use(int bytes)
  */
 static void *get_data(unsigned long size)
 {
-	git_zstream stream;
+	struct git_zstream stream;
 	unsigned long bufsize = dry_run && size > 8192 ? 8192 : size;
 	void *buf = xmallocz(bufsize);
 
@@ -352,7 +352,7 @@ static void unpack_non_delta_entry(enum object_type type, unsigned long size,
 }
 
 struct input_zstream_data {
-	git_zstream *zstream;
+	struct git_zstream *zstream;
 	unsigned char buf[8192];
 	int status;
 };
@@ -361,7 +361,7 @@ static const void *feed_input_zstream(struct input_stream *in_stream,
 				      unsigned long *readlen)
 {
 	struct input_zstream_data *data = in_stream->data;
-	git_zstream *zstream = data->zstream;
+	struct git_zstream *zstream = data->zstream;
 	void *in = fill(1);
 
 	if (in_stream->is_finished) {
@@ -385,7 +385,7 @@ static const void *feed_input_zstream(struct input_stream *in_stream,
 
 static void stream_blob(unsigned long size, unsigned nr)
 {
-	git_zstream zstream = { 0 };
+	struct git_zstream zstream = { 0 };
 	struct input_zstream_data data = { 0 };
 	struct input_stream in_stream = {
 		.read = feed_input_zstream,
diff --git a/cache.h b/cache.h
index ac5ab4ef9d3..797f8e4edae 100644
--- a/cache.h
+++ b/cache.h
@@ -18,7 +18,7 @@
 #include "repository.h"
 #include "mem-pool.h"
 
-typedef struct git_zstream {
+struct git_zstream {
 	z_stream z;
 	unsigned long avail_in;
 	unsigned long avail_out;
@@ -26,21 +26,21 @@ typedef struct git_zstream {
 	unsigned long total_out;
 	unsigned char *next_in;
 	unsigned char *next_out;
-} git_zstream;
-
-void git_inflate_init(git_zstream *);
-void git_inflate_init_gzip_only(git_zstream *);
-void git_inflate_end(git_zstream *);
-int git_inflate(git_zstream *, int flush);
-
-void git_deflate_init(git_zstream *, int level);
-void git_deflate_init_gzip(git_zstream *, int level);
-void git_deflate_init_raw(git_zstream *, int level);
-void git_deflate_end(git_zstream *);
-int git_deflate_abort(git_zstream *);
-int git_deflate_end_gently(git_zstream *);
-int git_deflate(git_zstream *, int flush);
-unsigned long git_deflate_bound(git_zstream *, unsigned long);
+};
+
+void git_inflate_init(struct git_zstream *);
+void git_inflate_init_gzip_only(struct git_zstream *);
+void git_inflate_end(struct git_zstream *);
+int git_inflate(struct git_zstream *, int flush);
+
+void git_deflate_init(struct git_zstream *, int level);
+void git_deflate_init_gzip(struct git_zstream *, int level);
+void git_deflate_init_raw(struct git_zstream *, int level);
+void git_deflate_end(struct git_zstream *);
+int git_deflate_abort(struct git_zstream *);
+int git_deflate_end_gently(struct git_zstream *);
+int git_deflate(struct git_zstream *, int flush);
+unsigned long git_deflate_bound(struct git_zstream *, unsigned long);
 
 #if defined(DT_UNKNOWN) && !defined(NO_D_TYPE_IN_DIRENT)
 #define DTYPE(de)	((de)->d_type)
@@ -1372,7 +1372,7 @@ enum unpack_loose_header_result {
 	ULHR_BAD,
 	ULHR_TOO_LONG,
 };
-enum unpack_loose_header_result unpack_loose_header(git_zstream *stream,
+enum unpack_loose_header_result unpack_loose_header(struct git_zstream *stream,
 						    unsigned char *map,
 						    unsigned long mapsize,
 						    void *buffer,
Previous: Eric SunshineNext: Eric Sunshine
Message 6 of 13 in “unpack-objects: fix compilation warning/error due to missing braces”
  1. unpack-objects: fix compilation warning/error due to missing bracesEric Sunshine, Jul 10, 2022
  2. Han XinJul 11, 2022
  3. Eric SunshineJul 11, 2022
  4. Junio C HamanoJul 11, 2022
  5. Eric SunshineJul 12, 2022
  6. Ævar Arnfjörð BjarmasonJul 12, 2022
  7. Eric SunshineJul 12, 2022
  8. Jeff KingJul 12, 2022
  9. Eric SunshineJul 12, 2022
  10. Ævar Arnfjörð BjarmasonJul 12, 2022
  11. Jeff KingJul 14, 2022
  12. Ævar Arnfjörð BjarmasonJul 15, 2022
  13. Junio C HamanoJul 12, 2022

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.