{"thread":{"id":"58237","subject":"[PATCH] merge-file: fix build warning with gcc 4.8.5","startedAt":"2022-07-29T09:06:20Z","lastAt":"2022-07-30T23:50:06Z","messageCount":11,"participants":["Đoàn Trần Công Danh","Junio C Hamano","Jeff King","Eric Sunshine","brian m. carlson"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"460190","messageId":"365e01e93dce582e9d926e83bdc6891310d22699.1659084832.git.congdanhqx@gmail.com","threadId":"58237","inReplyTo":null,"subject":"[PATCH] merge-file: fix build warning with gcc 4.8.5","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2022-07-29T09:05:55Z","receivedAt":"2022-07-29T09:06:20Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"mmfs is an array of struct, xmparamt_t's first field is a struct,\nmmfs's element and xmparamt_t's first field must be initialised\nwith {0}.\n\nSigned-off-by: Đoàn Trần Công Danh \n---\n\n This warning is available in master\n\n builtin/merge-file.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/merge-file.c b/builtin/merge-file.c\nindex c923bbf2ab..607c3d3f9e 100644\n--- a/builtin/merge-file.c\n+++ b/builtin/merge-file.c\n@@ -26,9 +26,9 @@ static int label_cb(const struct option *opt, const char *arg, int unset)\n int cmd_merge_file(int argc, const char **argv, const char *prefix)\n {\n \tconst char *names[3] = { 0 };\n-\tmmfile_t mmfs[3] = { 0 };\n+\tmmfile_t mmfs[3] = { { 0 } };\n \tmmbuffer_t result = { 0 };\n-\txmparam_t xmp = { 0 };\n+\txmparam_t xmp = { { 0 } };\n \tint ret = 0, i = 0, to_stdout = 0;\n \tint quiet = 0;\n \tstruct option options[] = {\n-- \n2.37.1.560.gdfb9273964\n\n"},{"id":"460228","messageId":"xmqqbkt7api9.fsf@gitster.g","threadId":"58237","inReplyTo":"365e01e93dce582e9d926e83bdc6891310d22699.1659084832.git.congdanhqx@gmail.com","subject":"Re: [PATCH] merge-file: fix build warning with gcc 4.8.5","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-29T15:48:46Z","receivedAt":"2022-07-29T15:48:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh writes:\n\n> mmfs is an array of struct, xmparamt_t's first field is a struct,\n> mmfs's element and xmparamt_t's first field must be initialised\n> with {0}.\n>\n> Signed-off-by: Đoàn Trần Công Danh \n> ---\n>\n> This warning is available in master\n\nThanks, but didn't we discuss this recently and decided that such\n(false) complaints by ancient compiler is not worth catering to by\ndeliberately deviating the \"zero initializing a struct is always\ndone via { 0 }\" idiom?\n\ncf. https://lore.kernel.org/git/CAPig+cSgNB=SzAZLhXvteSYmy0HvJh+qWHMYyBxcX_EA9__u4A@mail.gmail.com/\n\nI think the concensus was that we should squelch the false warning\non older compilers with -Wno-missing-braces, but then the discussion\nhas stalled by a suggestion to introduce a way to detect older\ncompilers that is different from how we do so at the same time, and\nwent nowhere.\n\nHopefully we can add a simple -Wno-* without waiting for whole\nconfig.mak thing getting revamped this time?\n\n\n"},{"id":"460265","messageId":"YuQ60ZUPBHAVETD7@coredump.intra.peff.net","threadId":"58237","inReplyTo":"xmqqbkt7api9.fsf@gitster.g","subject":"Re: [PATCH] merge-file: fix build warning with gcc 4.8.5","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-07-29T19:53:53Z","receivedAt":"2022-07-29T19:54:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 29, 2022 at 08:48:46AM -0700, Junio C Hamano wrote:\n\n> I think the concensus was that we should squelch the false warning\n> on older compilers with -Wno-missing-braces, but then the discussion\n> has stalled by a suggestion to introduce a way to detect older\n> compilers that is different from how we do so at the same time, and\n> went nowhere.\n> \n> Hopefully we can add a simple -Wno-* without waiting for whole\n> config.mak thing getting revamped this time?\n\nPerhaps this?\n\n-- >8 --\nSubject: [PATCH] config.mak.dev: squelch -Wno-missing-braces for older gcc\n\nVersions of gcc prior to 4.9 complain about an initialization like:\n\n struct inner { int x; };\n struct outer { struct inner; };\n struct outer foo = { 0 };\n\nand insist on:\n\n struct outer foo = { { 0 } };\n\nNewer compilers handle this just fine. And ignoring the window even on\nolder compilers is fine; the resulting code is correct, but we just get\ncaught by -Werror.\n\nLet's relax this for older compilers to make developer lives easier (we\ndon't care much about non-developers on old compilers; they may see a\nwarning, but it won't stop compilation).\n\nSigned-off-by: Jeff King \n---\nTested on a debian jessie chroot using gcc-4.8 and 4.9. Though note that\nyou also need to manually specify -std=gnu99 to get it to work at all\nwith those compilers these days! So I kind of wonder if it's even worth\ncatering to their warnings automatically.\n\n config.mak.dev | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/config.mak.dev b/config.mak.dev\nindex 335efd4620..b9878a4994 100644\n--- a/config.mak.dev\n+++ b/config.mak.dev\n@@ -59,9 +59,13 @@ endif\n \n # uninitialized warnings on gcc 4.9.2 in xdiff/xdiffi.c and config.c\n # not worth fixing since newer compilers correctly stop complaining\n+#\n+# Likwise, gcc older than 4.9 complains about initializing a\n+# struct-within-a-struct using just \"{ 0 }\"\n ifneq ($(filter gcc4,$(COMPILER_FEATURES)),)\n ifeq ($(filter gcc5,$(COMPILER_FEATURES)),)\n DEVELOPER_CFLAGS += -Wno-uninitialized\n+DEVELOPER_CFLAGS += -Wno-missing-braces\n endif\n endif\n \n-- \n2.37.1.804.g1775fa20e0\n\n"},{"id":"460266","messageId":"YuQ7E4qoac8YUpJ1@coredump.intra.peff.net","threadId":"58237","inReplyTo":"YuQ60ZUPBHAVETD7@coredump.intra.peff.net","subject":"Re: [PATCH] merge-file: fix build warning with gcc 4.8.5","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-07-29T19:54:59Z","receivedAt":"2022-07-29T19:55:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 29, 2022 at 03:53:54PM -0400, Jeff King wrote:\n\n> # uninitialized warnings on gcc 4.9.2 in xdiff/xdiffi.c and config.c\n> # not worth fixing since newer compilers correctly stop complaining\n> +#\n> +# Likwise, gcc older than 4.9 complains about initializing a\n> +# struct-within-a-struct using just \"{ 0 }\"\n\ns/Lik/Like/\n\nThat's what I get for last-minute editing. :)\n\n-Peff\n"},{"id":"460271","messageId":"CAPig+cSiAEgopj1zwR4kUG=ZLJQrNAVj0P5g4veWYD-ERr=irw@mail.gmail.com","threadId":"58237","inReplyTo":"YuQ60ZUPBHAVETD7@coredump.intra.peff.net","subject":"Re: [PATCH] merge-file: fix build warning with gcc 4.8.5","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2022-07-29T20:48:55Z","receivedAt":"2022-07-29T20:49:10Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jul 29, 2022 at 3:53 PM Jeff King wrote:\n> Subject: [PATCH] config.mak.dev: squelch -Wno-missing-braces for older gcc\n>\n> Versions of gcc prior to 4.9 complain about an initialization like:\n>\n> struct inner { int x; };\n> struct outer { struct inner; };\n> struct outer foo = { 0 };\n>\n> and insist on:\n>\n> struct outer foo = { { 0 } };\n>\n> Newer compilers handle this just fine. And ignoring the window even on\n> older compilers is fine; the resulting code is correct, but we just get\n> caught by -Werror.\n>\n> Let's relax this for older compilers to make developer lives easier (we\n> don't care much about non-developers on old compilers; they may see a\n> warning, but it won't stop compilation).\n>\n> Signed-off-by: Jeff King \n\nLeaving aside for the moment the problem with Apple's oddball invented\nversion numbers for `clang`, should this patch also take older `clang`\nversions into consideration rather than focusing only on `gcc`? (Of\ncourse, `clang` could be dealt with in a separate patch if you'd\nrather not worry about it here.)\n\n> diff --git a/config.mak.dev b/config.mak.dev\n> index 335efd4620..b9878a4994 100644\n> --- a/config.mak.dev\n> +++ b/config.mak.dev\n> @@ -59,9 +59,13 @@ endif\n>\n> # uninitialized warnings on gcc 4.9.2 in xdiff/xdiffi.c and config.c\n> # not worth fixing since newer compilers correctly stop complaining\n> +#\n> +# Likwise, gcc older than 4.9 complains about initializing a\n> +# struct-within-a-struct using just \"{ 0 }\"\n> ifneq ($(filter gcc4,$(COMPILER_FEATURES)),)\n> ifeq ($(filter gcc5,$(COMPILER_FEATURES)),)\n> DEVELOPER_CFLAGS += -Wno-uninitialized\n> +DEVELOPER_CFLAGS += -Wno-missing-braces\n> endif\n> endif\n"},{"id":"460272","messageId":"YuRKe/LdNs0u7G2w@coredump.intra.peff.net","threadId":"58237","inReplyTo":"CAPig+cSiAEgopj1zwR4kUG=ZLJQrNAVj0P5g4veWYD-ERr=irw@mail.gmail.com","subject":"Re: [PATCH] merge-file: fix build warning with gcc 4.8.5","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-07-29T21:00:43Z","receivedAt":"2022-07-29T21:00:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 29, 2022 at 04:48:55PM -0400, Eric Sunshine wrote:\n\n> Leaving aside for the moment the problem with Apple's oddball invented\n> version numbers for `clang`, should this patch also take older `clang`\n> versions into consideration rather than focusing only on `gcc`? (Of\n> course, `clang` could be dealt with in a separate patch if you'd\n> rather not worry about it here.)\n\nI was just fixing the reported gcc issue, and forgot totally that clang\nhad been mentioned in previous rounds. I'd be happy to just see a clang\npatch on top of this once somebody figures out the right versions (but\nit may be impossible without figuring out the oddball Apple thing).\n\n-Peff\n"},{"id":"460275","messageId":"xmqqzggr4nqa.fsf@gitster.g","threadId":"58237","inReplyTo":"YuRKe/LdNs0u7G2w@coredump.intra.peff.net","subject":"Re: [PATCH] merge-file: fix build warning with gcc 4.8.5","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-29T21:23:41Z","receivedAt":"2022-07-29T21:23:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Fri, Jul 29, 2022 at 04:48:55PM -0400, Eric Sunshine wrote:\n>\n>> Leaving aside for the moment the problem with Apple's oddball invented\n>> version numbers for `clang`, should this patch also take older `clang`\n>> versions into consideration rather than focusing only on `gcc`? (Of\n>> course, `clang` could be dealt with in a separate patch if you'd\n>> rather not worry about it here.)\n>\n> I was just fixing the reported gcc issue, and forgot totally that clang\n> had been mentioned in previous rounds. I'd be happy to just see a clang\n> patch on top of this once somebody figures out the right versions (but\n> it may be impossible without figuring out the oddball Apple thing).\n\nI am willing to say that we do not care about \"oddball Apple thing\"\nand have developers on that platform to propose how to handle their\ncompiler.\n\nIn any case, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119\nseems to indicate that given enough time this problem will\ndisappear, so I'd refrain from suggesting to use -Wno-missing-braces\neverywhere.\n\nThanks.\n"},{"id":"460293","messageId":"YuR5Jado2LrtZUjP@danh.dev","threadId":"58237","inReplyTo":"YuQ60ZUPBHAVETD7@coredump.intra.peff.net","subject":"Re: [PATCH] merge-file: fix build warning with gcc 4.8.5","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2022-07-30T00:19:49Z","receivedAt":"2022-07-30T00:19:56Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2022-07-29 15:53:53-0400, Jeff King wrote:\n> On Fri, Jul 29, 2022 at 08:48:46AM -0700, Junio C Hamano wrote:\n> \n> > I think the concensus was that we should squelch the false warning\n> > on older compilers with -Wno-missing-braces, but then the discussion\n> > has stalled by a suggestion to introduce a way to detect older\n> > compilers that is different from how we do so at the same time, and\n> > went nowhere.\n> > \n> > Hopefully we can add a simple -Wno-* without waiting for whole\n> > config.mak thing getting revamped this time?\n> \n> Perhaps this?\n> \n> -- >8 --\n> Subject: [PATCH] config.mak.dev: squelch -Wno-missing-braces for older gcc\n> \n> Versions of gcc prior to 4.9 complain about an initialization like:\n> \n> struct inner { int x; };\n> struct outer { struct inner; };\n> struct outer foo = { 0 };\n> \n> and insist on:\n> \n> struct outer foo = { { 0 } };\n> \n> Newer compilers handle this just fine. And ignoring the window even on\n> older compilers is fine; the resulting code is correct, but we just get\n> caught by -Werror.\n> \n> Let's relax this for older compilers to make developer lives easier (we\n> don't care much about non-developers on old compilers; they may see a\n> warning, but it won't stop compilation).\n> \n> Signed-off-by: Jeff King \n> ---\n> Tested on a debian jessie chroot using gcc-4.8 and 4.9. Though note that\n> you also need to manually specify -std=gnu99 to get it to work at all\n> with those compilers these days! So I kind of wonder if it's even worth\n> catering to their warnings automatically.\n\nWell, config.mak.uname automatically adds -std=c99 for RHEL 7 and\nCentOS7. Can we add the same things for Debian? Or should we just\nremove both?\n\n\n-- \nDanh\n"},{"id":"460296","messageId":"YuSL/2N29cCy9Dzd@coredump.intra.peff.net","threadId":"58237","inReplyTo":"YuR5Jado2LrtZUjP@danh.dev","subject":"Re: [PATCH] merge-file: fix build warning with gcc 4.8.5","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-07-30T01:40:32Z","receivedAt":"2022-07-30T01:41:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 30, 2022 at 07:19:49AM +0700, Đoàn Trần Công Danh wrote:\n\n> > Tested on a debian jessie chroot using gcc-4.8 and 4.9. Though note that\n> > you also need to manually specify -std=gnu99 to get it to work at all\n> > with those compilers these days! So I kind of wonder if it's even worth\n> > catering to their warnings automatically.\n> \n> Well, config.mak.uname automatically adds -std=c99 for RHEL 7 and\n> CentOS7. Can we add the same things for Debian? Or should we just\n> remove both?\n\nAh, I didn't know about that. No, I don't think there's any reason to\nremove them. If people are able to compile out of the box there, then my\npatch to config.mak.dev may be worth doing.\n\nAs far as adding a similar config.mak.uname thing for Debian, I don't\nhave a strong opinion. Jessie is far beyond being supported, so I'd\nprobably wait to see if somebody who actually cares proposes a patch.\n\n-Peff\n"},{"id":"460297","messageId":"YuSNhhRVpg2oT6ry@tapette.crustytoothpaste.net","threadId":"58237","inReplyTo":"YuR5Jado2LrtZUjP@danh.dev","subject":"Re: [PATCH] merge-file: fix build warning with gcc 4.8.5","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2022-07-30T01:46:46Z","receivedAt":"2022-07-30T01:46:51Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2022-07-30 at 00:19:49, Đoàn Trần Công Danh wrote:\n> On 2022-07-29 15:53:53-0400, Jeff King wrote:\n> > Tested on a debian jessie chroot using gcc-4.8 and 4.9. Though note that\n> > you also need to manually specify -std=gnu99 to get it to work at all\n> > with those compilers these days! So I kind of wonder if it's even worth\n> > catering to their warnings automatically.\n> \n> Well, config.mak.uname automatically adds -std=c99 for RHEL 7 and\n> CentOS7. Can we add the same things for Debian? Or should we just\n> remove both?\n\nI don't think we can do that, since Debian kernels don't include a\ndistinguishing pattern like that[0]. Also, Debian jessie doesn't have a\nfull set of security support, unlike CentOS 7, and thus I would argue we\nprobably wouldn't want to support it anyway.\n\nMy guess is that Peff used jessie because he uses Debian and it's easier\nfor him to set up than CentOS 7, not because we should use it as an\nintentional target.\n\nPersonally, although I don't use RHEL and company in either my personal\nor professional life anymore, I think it's probably worth providing a\nmodicum of support to because they're very common, at least as long as\nthere are freely available clones with security support (e.g., CentOS\nand Rocky Linux) that we can test against.\n\nAll that to say that I think we don't need to change config.mak.uname\nand can rely on folks just setting -std=c99 if need be.\n\n[0] Okay, there is a pattern, but it doesn't include a fixed string and\nneither shell nor make are ideal for pattern matching on it.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"460316","messageId":"YuXDqRNQSeyQXWGj@coredump.intra.peff.net","threadId":"58237","inReplyTo":"YuSNhhRVpg2oT6ry@tapette.crustytoothpaste.net","subject":"Re: [PATCH] merge-file: fix build warning with gcc 4.8.5","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-07-30T23:50:01Z","receivedAt":"2022-07-30T23:50:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 30, 2022 at 01:46:46AM +0000, brian m. carlson wrote:\n\n> > Well, config.mak.uname automatically adds -std=c99 for RHEL 7 and\n> > CentOS7. Can we add the same things for Debian? Or should we just\n> > remove both?\n> \n> I don't think we can do that, since Debian kernels don't include a\n> distinguishing pattern like that[0]. Also, Debian jessie doesn't have a\n> full set of security support, unlike CentOS 7, and thus I would argue we\n> probably wouldn't want to support it anyway.\n\nThe check really ought to be using the compiler version and not uname\nanyway. But outside of DEVELOPER=1, we don't (yet) do any probing of the\ncompiler.\n\n> My guess is that Peff used jessie because he uses Debian and it's easier\n> for him to set up than CentOS 7, not because we should use it as an\n> intentional target.\n\nYep, exactly.\n\n> Personally, although I don't use RHEL and company in either my personal\n> or professional life anymore, I think it's probably worth providing a\n> modicum of support to because they're very common, at least as long as\n> there are freely available clones with security support (e.g., CentOS\n> and Rocky Linux) that we can test against.\n> \n> All that to say that I think we don't need to change config.mak.uname\n> and can rely on folks just setting -std=c99 if need be.\n\nAgreed. I only brought it up as a \"gee, is anybody even using this?\"\nhead-scratcher. I just wasn't aware of the CentOS workaround.\n\n-Peff\n"}]}