| Message ID | c4fac2ae9d10bc426cb26e4a102b808549696763.1587648870.git.congdanhqx@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fix make sparse warning | expand |
On 23/04/2020 14:47, Đoàn Trần Công Danh wrote: > Fix warning from `make sparse`. You may want to split the changes to 't/helper/test-parse-pathspec-file.c' into its own patch, since those changes are not potentially controversial. The remainder of this patch follows a pattern that I used in a patch that was rejected; see https://public-inbox.org/git/e6796c60-a870-e761-3b07-b680f934c537@ramsayjones.plus.com/, et seq. Actually, I have a patch somewhere which suppressed the sparse warning for the '= { 0 }' token sequence used in these initializations. However, I don't seem to be able to find them at the moment! :( [Luc, this topic came up on the sparse and kernel mailing-lists at one point, but I didn't get around to posting my patch to the list - something came up. Hopefully, I will find some time to find it and post it soon.] ATB, Ramsay Jones > > Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> > --- > add-interactive.c | 2 +- > builtin/fmt-merge-msg.c | 2 +- > log-tree.c | 4 ++-- > range-diff.c | 2 +- > t/helper/test-parse-pathspec-file.c | 6 +++--- > 5 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/add-interactive.c b/add-interactive.c > index 29cd2fe020..b8983838b9 100644 > --- a/add-interactive.c > +++ b/add-interactive.c > @@ -526,7 +526,7 @@ static int get_modified_files(struct repository *r, > > for (i = 0; i < 2; i++) { > struct rev_info rev; > - struct setup_revision_opt opt = { 0 }; > + struct setup_revision_opt opt = { NULL }; > > if (filter == INDEX_ONLY) > s.mode = (i == 0) ? FROM_INDEX : FROM_WORKTREE; > diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c > index 172dfbd852..f4376bccef 100644 > --- a/builtin/fmt-merge-msg.c > +++ b/builtin/fmt-merge-msg.c > @@ -494,7 +494,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out) > enum object_type type; > unsigned long size, len; > char *buf = read_object_file(oid, &type, &size); > - struct signature_check sigc = { 0 }; > + struct signature_check sigc = { NULL }; > struct strbuf sig = STRBUF_INIT; > > if (!buf || type != OBJ_TAG) > diff --git a/log-tree.c b/log-tree.c > index 0064788b25..ca721150d4 100644 > --- a/log-tree.c > +++ b/log-tree.c > @@ -449,7 +449,7 @@ static void show_signature(struct rev_info *opt, struct commit *commit) > { > struct strbuf payload = STRBUF_INIT; > struct strbuf signature = STRBUF_INIT; > - struct signature_check sigc = { 0 }; > + struct signature_check sigc = { NULL }; > int status; > > if (parse_signed_commit(commit, &payload, &signature) <= 0) > @@ -496,7 +496,7 @@ static int show_one_mergetag(struct commit *commit, > struct object_id oid; > struct tag *tag; > struct strbuf verify_message; > - struct signature_check sigc = { 0 }; > + struct signature_check sigc = { NULL }; > int status, nth; > size_t payload_size; > > diff --git a/range-diff.c b/range-diff.c > index f745567cf6..71dcd947c5 100644 > --- a/range-diff.c > +++ b/range-diff.c > @@ -107,7 +107,7 @@ static int read_patches(const char *range, struct string_list *list, > } > > if (starts_with(line, "diff --git")) { > - struct patch patch = { 0 }; > + struct patch patch = { NULL }; > struct strbuf root = STRBUF_INIT; > int linenr = 0; > > diff --git a/t/helper/test-parse-pathspec-file.c b/t/helper/test-parse-pathspec-file.c > index 02f4ccfd2a..b3e08cef4b 100644 > --- a/t/helper/test-parse-pathspec-file.c > +++ b/t/helper/test-parse-pathspec-file.c > @@ -6,7 +6,7 @@ > int cmd__parse_pathspec_file(int argc, const char **argv) > { > struct pathspec pathspec; > - const char *pathspec_from_file = 0; > + const char *pathspec_from_file = NULL; > int pathspec_file_nul = 0, i; > > static const char *const usage[] = { > @@ -20,9 +20,9 @@ int cmd__parse_pathspec_file(int argc, const char **argv) > OPT_END() > }; > > - parse_options(argc, argv, 0, options, usage, 0); > + parse_options(argc, argv, NULL, options, usage, 0); > > - parse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file, > + parse_pathspec_file(&pathspec, 0, 0, NULL, pathspec_from_file, > pathspec_file_nul); > > for (i = 0; i < pathspec.nr; i++) >
Ramsay Jones <ramsay@ramsayjones.plus.com> writes: > Actually, I have a patch somewhere which suppressed the sparse warning > for the '= { 0 }' token sequence used in these initializations. However, > I don't seem to be able to find them at the moment! :( AHHHHHhhhhhhhh. Thanks for reminding. Yeah, I recall that the concensus of those who were vocal in that old discussion [*1*] was that "= { 0 }" should be taken as an idiom and should not be subject to s/0/NULL/ conversion. > [Luc, this topic came up on the sparse and kernel mailing-lists at one > point, but I didn't get around to posting my patch to the list - something > came up. Hopefully, I will find some time to find it and post it soon.] [References] *1* https://lore.kernel.org/git/e6796c60-a870-e761-3b07-b680f934c537@ramsayjones.plus.com/
On 2020-04-23 17:54:30-0700, Junio C Hamano <gitster@pobox.com> wrote: > Ramsay Jones <ramsay@ramsayjones.plus.com> writes: > > > Actually, I have a patch somewhere which suppressed the sparse warning > > for the '= { 0 }' token sequence used in these initializations. However, > > I don't seem to be able to find them at the moment! :( > > AHHHHHhhhhhhhh. > > Thanks for reminding. Yeah, I recall that the concensus of those > who were vocal in that old discussion [*1*] was that "= { 0 }" > should be taken as an idiom and should not be subject to s/0/NULL/ > conversion. So, to follow that idiom, this patch should be changed, too? https://lore.kernel.org/git/nycvar.QRO.7.76.6.2004101604210.46@tvgsbejvaqbjf.bet/ +Cc: Dscho and Han-Wen > > [Luc, this topic came up on the sparse and kernel mailing-lists at one > > point, but I didn't get around to posting my patch to the list - something > > came up. Hopefully, I will find some time to find it and post it soon.] > > > [References] > > *1* > > https://lore.kernel.org/git/e6796c60-a870-e761-3b07-b680f934c537@ramsayjones.plus.com/
Danh Doan <congdanhqx@gmail.com> writes: > So, to follow that idiom, this patch should be changed, too? > https://lore.kernel.org/git/nycvar.QRO.7.76.6.2004101604210.46@tvgsbejvaqbjf.bet/ I didn't look at that patch carefully, but it seems that it changes struct foo variable_to_be_initialized = {}; into various forms, e.g. struct foo variable_to_be_initialized = { NULL }; struct foo variable_to_be_initialized = { { NULL } }; depending on the actual shape of 'foo', and that old thread explains that it is sufficient to consistently write "= { 0 };". Having said that, if I recall correctly, Dscho suggested even larger style fixes to the code, so if that were to happen, the above initializer fix may just be lost in the noise ;-) Thanks. > +Cc: Dscho and Han-Wen > >> > [Luc, this topic came up on the sparse and kernel mailing-lists at one >> > point, but I didn't get around to posting my patch to the list - something >> > came up. Hopefully, I will find some time to find it and post it soon.] >> >> >> [References] >> >> *1* >> >> https://lore.kernel.org/git/e6796c60-a870-e761-3b07-b680f934c537@ramsayjones.plus.com/
On Thu, Apr 23, 2020 at 05:54:30PM -0700, Junio C Hamano wrote: > Ramsay Jones <ramsay@ramsayjones.plus.com> writes: > > > Actually, I have a patch somewhere which suppressed the sparse warning > > for the '= { 0 }' token sequence used in these initializations. However, > > I don't seem to be able to find them at the moment! :( > > AHHHHHhhhhhhhh. > > Thanks for reminding. Yeah, I recall that the concensus of those > who were vocal in that old discussion [*1*] was that "= { 0 }" > should be taken as an idiom and should not be subject to s/0/NULL/ > conversion. > > > [Luc, this topic came up on the sparse and kernel mailing-lists at one > > point, but I didn't get around to posting my patch to the list - something > > came up. Hopefully, I will find some time to find it and post it soon.] > > > [References] > > *1* > > https://lore.kernel.org/git/e6796c60-a870-e761-3b07-b680f934c537@ramsayjones.plus.com/ Sorry for the late reply. I hadn't seen this old discussion but I vaguely remember 2 emails about this problem, probaly on lkml, without a real discussion, but where the opinion was the opposite. Personally, I prefer '= { }' but yes it's not legit and it seems that some compilers don't like it. I'll be glad to add an option to Sparse to shut up the warnings now given by '{ 0 }'. -- Luc
diff --git a/add-interactive.c b/add-interactive.c index 29cd2fe020..b8983838b9 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -526,7 +526,7 @@ static int get_modified_files(struct repository *r, for (i = 0; i < 2; i++) { struct rev_info rev; - struct setup_revision_opt opt = { 0 }; + struct setup_revision_opt opt = { NULL }; if (filter == INDEX_ONLY) s.mode = (i == 0) ? FROM_INDEX : FROM_WORKTREE; diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c index 172dfbd852..f4376bccef 100644 --- a/builtin/fmt-merge-msg.c +++ b/builtin/fmt-merge-msg.c @@ -494,7 +494,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out) enum object_type type; unsigned long size, len; char *buf = read_object_file(oid, &type, &size); - struct signature_check sigc = { 0 }; + struct signature_check sigc = { NULL }; struct strbuf sig = STRBUF_INIT; if (!buf || type != OBJ_TAG) diff --git a/log-tree.c b/log-tree.c index 0064788b25..ca721150d4 100644 --- a/log-tree.c +++ b/log-tree.c @@ -449,7 +449,7 @@ static void show_signature(struct rev_info *opt, struct commit *commit) { struct strbuf payload = STRBUF_INIT; struct strbuf signature = STRBUF_INIT; - struct signature_check sigc = { 0 }; + struct signature_check sigc = { NULL }; int status; if (parse_signed_commit(commit, &payload, &signature) <= 0) @@ -496,7 +496,7 @@ static int show_one_mergetag(struct commit *commit, struct object_id oid; struct tag *tag; struct strbuf verify_message; - struct signature_check sigc = { 0 }; + struct signature_check sigc = { NULL }; int status, nth; size_t payload_size; diff --git a/range-diff.c b/range-diff.c index f745567cf6..71dcd947c5 100644 --- a/range-diff.c +++ b/range-diff.c @@ -107,7 +107,7 @@ static int read_patches(const char *range, struct string_list *list, } if (starts_with(line, "diff --git")) { - struct patch patch = { 0 }; + struct patch patch = { NULL }; struct strbuf root = STRBUF_INIT; int linenr = 0; diff --git a/t/helper/test-parse-pathspec-file.c b/t/helper/test-parse-pathspec-file.c index 02f4ccfd2a..b3e08cef4b 100644 --- a/t/helper/test-parse-pathspec-file.c +++ b/t/helper/test-parse-pathspec-file.c @@ -6,7 +6,7 @@ int cmd__parse_pathspec_file(int argc, const char **argv) { struct pathspec pathspec; - const char *pathspec_from_file = 0; + const char *pathspec_from_file = NULL; int pathspec_file_nul = 0, i; static const char *const usage[] = { @@ -20,9 +20,9 @@ int cmd__parse_pathspec_file(int argc, const char **argv) OPT_END() }; - parse_options(argc, argv, 0, options, usage, 0); + parse_options(argc, argv, NULL, options, usage, 0); - parse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file, + parse_pathspec_file(&pathspec, 0, 0, NULL, pathspec_from_file, pathspec_file_nul); for (i = 0; i < pathspec.nr; i++)
Fix warning from `make sparse`. Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> --- add-interactive.c | 2 +- builtin/fmt-merge-msg.c | 2 +- log-tree.c | 4 ++-- range-diff.c | 2 +- t/helper/test-parse-pathspec-file.c | 6 +++--- 5 files changed, 8 insertions(+), 8 deletions(-)