Re: [PATCH] unpack-objects: fix compilation warning/error due to missing braces
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2022-07-12 06:55:44
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2022-07-12 · Re: [PATCH] unpack-objects: fix compilation warning/error due to missing braces · Junio C Hamano <hidden>
- 2022-07-11 · Re: [PATCH] unpack-objects: fix compilation warning/error due to missing braces · Eric Sunshine <hidden>
- 2022-07-11 · Re: [PATCH] unpack-objects: fix compilation warning/error due to missing braces · Han Xin <hidden>
- 2022-07-10 · [PATCH] unpack-objects: fix compilation warning/error due to missing braces · Eric Sunshine <hidden>
On Tue, Jul 12 2022, Eric Sunshine wrote:
On Mon, Jul 11, 2022 at 12:38 AM Junio C Hamano [off-list ref] wrote:quoted
Eric Sunshine [off-list ref] writes:quoted
On Sun, Jul 10, 2022 at 10:00 PM Han Xin [off-list ref] wrote:quoted
On Sun, Jul 10, 2022 at 4:12 PM Eric Sunshine [off-list ref] wrote:quoted
[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,