{"thread":{"id":"43006","subject":"[PATCH 0/2] Patches to let Git build with GCC 6 and DEVELOPER=SureWhyNot","startedAt":"2016-08-04T16:07:15Z","lastAt":"2016-08-06T21:59:39Z","messageCount":17,"participants":["Johannes Schindelin","Junio C Hamano","René Scharfe","Johannes Sixt","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"293094","messageId":"cover.1470326812.git.johannes.schindelin@gmx.de","threadId":"43006","inReplyTo":null,"subject":"[PATCH 0/2] Patches to let Git build with GCC 6 and DEVELOPER=SureWhyNot","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-04T16:07:00Z","receivedAt":"2016-08-04T16:07:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"While working on some local setup to allow my poor little laptop to\nbuild and test the Windows-specific patches of Git for Windows on top of\nthe upstream branches, my development environment updated to use GCC 6,\nand these patches were required.\n\n\nJohannes Schindelin (2):\n nedmalloc: fix misleading indentation\n nedmalloc: work around overzealous GCC 6 warning\n\n compat/nedmalloc/nedmalloc.c | 11 +++++++----\n 1 file changed, 7 insertions(+), 4 deletions(-)\n\nPublished-As: https://github.com/dscho/git/releases/tag/gcc-6-v1\nFetch-It-Via: git fetch https://github.com/dscho/git gcc-6-v1\n-- \n2.9.0.281.g286a8d9\n\nbase-commit: 80460f513ebd7851953f5402dd9744236128b240\n"},{"id":"293096","messageId":"0e17a706c29932e2280a1bc1730f5064bbcfec57.1470326812.git.johannes.schindelin@gmx.de","threadId":"43006","inReplyTo":"cover.1470326812.git.johannes.schindelin@gmx.de","subject":"[PATCH 1/2] nedmalloc: fix misleading indentation","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-04T16:07:03Z","receivedAt":"2016-08-04T16:07:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Some code in nedmalloc is indented in a funny way that could be\nmisinterpreted as if a line after a for loop was included in the loop\nbody, when it is not.\n\nGCC 6 complains about this in DEVELOPER=YepSure mode.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/nedmalloc/nedmalloc.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c\nindex a0a16eb..677d1b2 100644\n--- a/compat/nedmalloc/nedmalloc.c\n+++ b/compat/nedmalloc/nedmalloc.c\n@@ -938,10 +938,10 @@ void **nedpindependent_comalloc(nedpool *p, size_t elems, size_t *sizes, void **\n \tvoid **ret;\n \tthreadcache *tc;\n \tint mymspace;\n- size_t i, *adjustedsizes=(size_t *) alloca(elems*sizeof(size_t));\n- if(!adjustedsizes) return 0;\n- for(i=0; i= 6 in that case and go on with our lives.\n\nSee https://gcc.gnu.org/gcc-6/porting_to.html for details.\n\nSigned-off-by: Johannes Schindelin \n---\n compat/nedmalloc/nedmalloc.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c\nindex 677d1b2..3f28c0b 100644\n--- a/compat/nedmalloc/nedmalloc.c\n+++ b/compat/nedmalloc/nedmalloc.c\n@@ -956,6 +956,9 @@ void **nedpindependent_comalloc(nedpool *p, size_t elems, size_t *sizes, void **\n char *strdup(const char *s1)\n {\n \tchar *s2 = 0;\n+#if __GNUC__ >= 6\n+#pragma GCC diagnostic ignored \"-Wnonnull-compare\"\n+#endif\n \tif (s1) {\n \t\tsize_t len = strlen(s1) + 1;\n \t\ts2 = malloc(len);\n-- \n2.9.0.281.g286a8d9\n"},{"id":"293107","messageId":"xmqqoa584eju.fsf@gitster.mtv.corp.google.com","threadId":"43006","inReplyTo":"57360f4885bdd5c36e190bea288f1e1f7f706071.1470326812.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-04T17:41:41Z","receivedAt":"2016-08-04T17:41:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> With GCC 6, the strdup() function is declared with the \"nonnull\"\n> attribute, stating that it is not allowed to pass a NULL value as\n> parameter.\n>\n> In nedmalloc()'s reimplementation of strdup(), Postel's Law is heeded\n> and NULL parameters are handled gracefully. GCC 6 complains about that\n> now because it thinks that NULL cannot be passed to strdup() anyway.\n>\n> Let's just shut up GCC >= 6 in that case and go on with our lives.\n>\n> See https://gcc.gnu.org/gcc-6/porting_to.html for details.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> compat/nedmalloc/nedmalloc.c | 3 +++\n> 1 file changed, 3 insertions(+)\n>\n> diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c\n> index 677d1b2..3f28c0b 100644\n> --- a/compat/nedmalloc/nedmalloc.c\n> +++ b/compat/nedmalloc/nedmalloc.c\n> @@ -956,6 +956,9 @@ void **nedpindependent_comalloc(nedpool *p, size_t elems, size_t *sizes, void **\n> char *strdup(const char *s1)\n> {\n> \tchar *s2 = 0;\n> +#if __GNUC__ >= 6\n> +#pragma GCC diagnostic ignored \"-Wnonnull-compare\"\n> +#endif\n> \tif (s1) {\n> \t\tsize_t len = strlen(s1) + 1;\n> \t\ts2 = malloc(len);\n\nIs it a common convention to place \"#pragma GCC diagnostic\"\nimmediately before the statement you want to affect, and have the\nsame pragma in effect until the end of the compilation unit?\n\nI know this function is at the end and it is not worth doing\npush/ignored/pop dance, and I assumed that it is the reason why we\nsee a single \"ignore from here on\", which is much simpler, but it is\nsomewhat distracting. It made me wonder if it makes it easier to\nread and less distracting to have these three lines in front of and\noutside the function definition, while thinking that it would have a\ndocumentation value to have it immediately before the statement you\nwant to affect. Help me convince myself that this is the best\nplace.\n\nThanks.\n\n"},{"id":"293144","messageId":"57A3BA26.5080601@web.de","threadId":"43006","inReplyTo":"57360f4885bdd5c36e190bea288f1e1f7f706071.1470326812.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2016-08-04T21:56:54Z","receivedAt":"2016-08-04T21:57:12Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 04.08.2016 um 18:07 schrieb Johannes Schindelin:\n> With GCC 6, the strdup() function is declared with the \"nonnull\"\n> attribute, stating that it is not allowed to pass a NULL value as\n> parameter.\n>\n> In nedmalloc()'s reimplementation of strdup(), Postel's Law is heeded\n> and NULL parameters are handled gracefully. GCC 6 complains about that\n> now because it thinks that NULL cannot be passed to strdup() anyway.\n>\n> Let's just shut up GCC >= 6 in that case and go on with our lives.\n\nThis version of strdup() is only compiled if nedmalloc is used instead\nof the system allocator. That means we can't rely on strdup() being\nable to take NULL -- some (most?) platforms won't like it. Removing\nthe NULL check would be a more general and overall easier way out, no?\n\nBut it should check the result of malloc() before copying.\n---\n compat/nedmalloc/nedmalloc.c | 8 +++-----\n 1 file changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c\nindex a0a16eb..cc18f0c 100644\n--- a/compat/nedmalloc/nedmalloc.c\n+++ b/compat/nedmalloc/nedmalloc.c\n@@ -955,12 +955,10 @@ void **nedpindependent_comalloc(nedpool *p, size_t \nelems, size_t *sizes, void **\n */\n char *strdup(const char *s1)\n {\n-\tchar *s2 = 0;\n-\tif (s1) {\n-\t\tsize_t len = strlen(s1) + 1;\n-\t\ts2 = malloc(len);\n+\tsize_t len = strlen(s1) + 1;\n+\tchar *s2 = malloc(len);\n+\tif (s2)\n \t\tmemcpy(s2, s1, len);\n-\t}\n \treturn s2;\n }\n #endif\n-- \n2.9.2\n\n"},{"id":"293147","messageId":"xmqqoa58yxin.fsf@gitster.mtv.corp.google.com","threadId":"43006","inReplyTo":"57A3BA26.5080601@web.de","subject":"Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-04T22:33:52Z","receivedAt":"2016-08-04T22:34:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> This version of strdup() is only compiled if nedmalloc is used instead\n> of the system allocator. That means we can't rely on strdup() being\n> able to take NULL -- some (most?) platforms won't like it. Removing\n> the NULL check would be a more general and overall easier way out, no?\n\nThe callers of this version must be prepared to call a version of\nstrdup() that does not accept NULL, so in a sense, passing NULL to\nthis function is already an error in the context of this project.\n\nThat sounds like a good rationale to take this more straight-forward\napproach.\n\n> But it should check the result of malloc() before copying.\n> ---\n> compat/nedmalloc/nedmalloc.c | 8 +++-----\n> 1 file changed, 3 insertions(+), 5 deletions(-)\n>\n> diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c\n> index a0a16eb..cc18f0c 100644\n> --- a/compat/nedmalloc/nedmalloc.c\n> +++ b/compat/nedmalloc/nedmalloc.c\n> @@ -955,12 +955,10 @@ void **nedpindependent_comalloc(nedpool *p,\n> size_t elems, size_t *sizes, void **\n> */\n> char *strdup(const char *s1)\n> {\n> -\tchar *s2 = 0;\n> -\tif (s1) {\n> -\t\tsize_t len = strlen(s1) + 1;\n> -\t\ts2 = malloc(len);\n> +\tsize_t len = strlen(s1) + 1;\n> +\tchar *s2 = malloc(len);\n> +\tif (s2)\n> \t\tmemcpy(s2, s1, len);\n> -\t}\n> \treturn s2;\n> }\n> #endif\n"},{"id":"293148","messageId":"xmqqk2fwyx8h.fsf@gitster.mtv.corp.google.com","threadId":"43006","inReplyTo":"57A3BA26.5080601@web.de","subject":"Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-04T22:39:58Z","receivedAt":"2016-08-04T22:40:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Let's try it this way. How about this as a replacement?\n\n-- >8 --\nFrom: Johannes Schindelin \nDate: Thu, 4 Aug 2016 18:07:08 +0200\nSubject: [PATCH] nedmalloc: work around overzealous GCC 6 warning\n\nWith GCC 6, the strdup() function is declared with the \"nonnull\"\nattribute, stating that it is not allowed to pass a NULL value as\nparameter.\n\nIn nedmalloc()'s reimplementation of strdup(), Postel's Law is heeded\nand NULL parameters are handled gracefully. GCC 6 complains about that\nnow because it thinks that NULL cannot be passed to strdup() anyway.\n\nBecause the callers in this project of strdup() must be prepared to\ncall any implementation of strdup() supplied by the platform, so it\nis pointless to pretend that it is OK to call it with NULL.\n\nRemove the conditional based on NULL-ness of the input; this\nsquelches the warning.\n\nSee https://gcc.gnu.org/gcc-6/porting_to.html for details.\n\nSigned-off-by: Johannes Schindelin \nHelped-by: René Scharfe \nSigned-off-by: Junio C Hamano \n---\n compat/nedmalloc/nedmalloc.c | 8 +++-----\n 1 file changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c\nindex 677d1b2..88cd78c 100644\n--- a/compat/nedmalloc/nedmalloc.c\n+++ b/compat/nedmalloc/nedmalloc.c\n@@ -955,12 +955,10 @@ void **nedpindependent_comalloc(nedpool *p, size_t elems, size_t *sizes, void **\n */\n char *strdup(const char *s1)\n {\n-\tchar *s2 = 0;\n-\tif (s1) {\n-\t\tsize_t len = strlen(s1) + 1;\n-\t\ts2 = malloc(len);\n+\tsize_t len = strlen(s1) + 1;\n+\ts2 = malloc(len);\n+\tif (s1)\n \t\tmemcpy(s2, s1, len);\n-\t}\n \treturn s2;\n }\n #endif\n-- \n2.9.2-766-gd7972a8\n\n"},{"id":"293159","messageId":"5ce2ec67-aa7d-0079-54fe-d73a70a46825@kdbg.org","threadId":"43006","inReplyTo":"xmqqk2fwyx8h.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-08-05T05:36:50Z","receivedAt":"2016-08-05T05:37:48Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 05.08.2016 um 00:39 schrieb Junio C Hamano:\n> @@ -955,12 +955,10 @@ void **nedpindependent_comalloc(nedpool *p, size_t elems, size_t *sizes, void **\n> */\n> char *strdup(const char *s1)\n> {\n> -\tchar *s2 = 0;\n> -\tif (s1) {\n> -\t\tsize_t len = strlen(s1) + 1;\n> -\t\ts2 = malloc(len);\n> +\tsize_t len = strlen(s1) + 1;\n> +\ts2 = malloc(len);\n> +\tif (s1)\n\nIt does not make sense to check s1 for NULL when it was passed to \nstrlen() earlier; strlen() does not accept NULL, either...\n\n> \t\tmemcpy(s2, s1, len);\n> -\t}\n> \treturn s2;\n> }\n> #endif\n\n-- Hannes\n\n"},{"id":"293160","messageId":"68c24062-7fcc-758f-373f-2f123c5eb78b@kdbg.org","threadId":"43006","inReplyTo":"5ce2ec67-aa7d-0079-54fe-d73a70a46825@kdbg.org","subject":"Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-08-05T05:40:48Z","receivedAt":"2016-08-05T05:40:55Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 05.08.2016 um 07:36 schrieb Johannes Sixt:\n> Am 05.08.2016 um 00:39 schrieb Junio C Hamano:\n>> @@ -955,12 +955,10 @@ void **nedpindependent_comalloc(nedpool *p,\n>> size_t elems, size_t *sizes, void **\n>> */\n>> char *strdup(const char *s1)\n>> {\n>> - char *s2 = 0;\n>> - if (s1) {\n>> - size_t len = strlen(s1) + 1;\n>> - s2 = malloc(len);\n>> + size_t len = strlen(s1) + 1;\n>> + s2 = malloc(len);\n>> + if (s1)\n>\n> It does not make sense to check s1 for NULL when it was passed to\n> strlen() earlier; strlen() does not accept NULL, either...\n\nOh! This is a typo. You meant to check s2 for NULL.\n\nAnd the declaration for s2 should remain, of course.\n\n>\n>> memcpy(s2, s1, len);\n>> - }\n>> return s2;\n>> }\n>> #endif\n\n-- Hannes\n\n"},{"id":"293161","messageId":"xmqq7fbvzqan.fsf@gitster.mtv.corp.google.com","threadId":"43006","inReplyTo":"68c24062-7fcc-758f-373f-2f123c5eb78b@kdbg.org","subject":"Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-05T06:24:32Z","receivedAt":"2016-08-05T06:24:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Oh! This is a typo. You meant to check s2 for NULL.\n>\n> And the declaration for s2 should remain, of course.\n\nYeah, the moral of the story is don't try to do something you do not\nusually do X-<.\n\nThe second try (the log message is the same).\n\nI do not know if we want to worry about st_add(1, strlen(s1))\noverflow around here, though.\n\n compat/nedmalloc/nedmalloc.c | 9 ++++-----\n 1 file changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c\nindex 677d1b2..2d4ef59 100644\n--- a/compat/nedmalloc/nedmalloc.c\n+++ b/compat/nedmalloc/nedmalloc.c\n@@ -955,12 +955,11 @@ void **nedpindependent_comalloc(nedpool *p, size_t elems, size_t *sizes, void **\n */\n char *strdup(const char *s1)\n {\n-\tchar *s2 = 0;\n-\tif (s1) {\n-\t\tsize_t len = strlen(s1) + 1;\n-\t\ts2 = malloc(len);\n+\tsize_t len = strlen(s1) + 1;\n+\tchar *s2 = malloc(len);\n+\n+\tif (s2)\n \t\tmemcpy(s2, s1, len);\n-\t}\n \treturn s2;\n }\n #endif\n"},{"id":"293163","messageId":"20160805074219.3vvgzaqnn6kvsh3s@sigill.intra.peff.net","threadId":"43006","inReplyTo":"xmqq7fbvzqan.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-05T07:42:19Z","receivedAt":"2016-08-05T07:42:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 04, 2016 at 11:24:32PM -0700, Junio C Hamano wrote:\n\n> I do not know if we want to worry about st_add(1, strlen(s1))\n> overflow around here, though.\n> [...]\n> +\tsize_t len = strlen(s1) + 1;\n\nI wondered that, too, but I don't think it's possible.\n\nTo overflow the size_t with \"+1\", strlen() must return the maximum value\nthat it can hold. But such a string would need one more byte than that,\nfor its trailing NUL. So assuming you cannot have a string that exceeds\nsize_t in the first place, I think it is impossible to overflow here.\n\n-Peff\n"},{"id":"293197","messageId":"alpine.DEB.2.20.1608051728380.5786@virtualbox","threadId":"43006","inReplyTo":"xmqqk2fwyx8h.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-05T15:34:49Z","receivedAt":"2016-08-05T15:35:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio & René,\n\nOn Thu, 4 Aug 2016, Junio C Hamano wrote:\n\n> Let's try it this way. How about this as a replacement?\n\nI like it (with the if (s2) test intead of if (s1), of course). But please\nrecord René as author, maybe mentioning myself with a \"Diagnosed-by:\"\nline.\n\nFWIW today's `pu` does pass the test suite without problems in my CI\nsetup.\n\nThis setup will from now on test next & pu in the Git for Windows SDK, and\nrebase Git for Windows' current master to git.git's maint, master, next &\npu, every morning after a weekday (unless I forget to turn on my laptop,\nthat is).\n\nOnce it stabilizes, I will figure out a way how to publish the logs,\nbecause playing Lt Tawney Madison gets old real quick.\n\nThanks,\nDscho"},{"id":"293198","messageId":"alpine.DEB.2.20.1608051736400.5786@virtualbox","threadId":"43006","inReplyTo":"xmqqoa584eju.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-05T15:38:46Z","receivedAt":"2016-08-05T15:39:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 4 Aug 2016, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > With GCC 6, the strdup() function is declared with the \"nonnull\"\n> > attribute, stating that it is not allowed to pass a NULL value as\n> > parameter.\n> >\n> > In nedmalloc()'s reimplementation of strdup(), Postel's Law is heeded\n> > and NULL parameters are handled gracefully. GCC 6 complains about that\n> > now because it thinks that NULL cannot be passed to strdup() anyway.\n> >\n> > Let's just shut up GCC >= 6 in that case and go on with our lives.\n> >\n> > See https://gcc.gnu.org/gcc-6/porting_to.html for details.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > compat/nedmalloc/nedmalloc.c | 3 +++\n> > 1 file changed, 3 insertions(+)\n> >\n> > diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c\n> > index 677d1b2..3f28c0b 100644\n> > --- a/compat/nedmalloc/nedmalloc.c\n> > +++ b/compat/nedmalloc/nedmalloc.c\n> > @@ -956,6 +956,9 @@ void **nedpindependent_comalloc(nedpool *p, size_t elems, size_t *sizes, void **\n> > char *strdup(const char *s1)\n> > {\n> > \tchar *s2 = 0;\n> > +#if __GNUC__ >= 6\n> > +#pragma GCC diagnostic ignored \"-Wnonnull-compare\"\n> > +#endif\n> > \tif (s1) {\n> > \t\tsize_t len = strlen(s1) + 1;\n> > \t\ts2 = malloc(len);\n> \n> Is it a common convention to place \"#pragma GCC diagnostic\"\n> immediately before the statement you want to affect, and have the\n> same pragma in effect until the end of the compilation unit?\n\nUh oh. This was a brain fart. I somehow confused the way pragmas work with\nthe way __attribute__s work. You are correct, of course, that the pragma\naffects the entire remainder of the file, not just this statement.\n\nLuckily, René came up with a much more elegant solution, so that Git's\nhistory does not have to shame me eternally.\n\nCiao,\nDscho"},{"id":"293205","messageId":"xmqqpopnxkfv.fsf@gitster.mtv.corp.google.com","threadId":"43006","inReplyTo":"alpine.DEB.2.20.1608051728380.5786@virtualbox","subject":"Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-05T16:13:56Z","receivedAt":"2016-08-05T16:14:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> This setup will from now on test next & pu in the Git for Windows SDK, and\n> rebase Git for Windows' current master to git.git's maint, master, next &\n> pu, every morning after a weekday (unless I forget to turn on my laptop,\n> that is).\n\nSounds really good. Thanks.\n\n"},{"id":"293260","messageId":"xmqqk2fuvpzm.fsf@gitster.mtv.corp.google.com","threadId":"43006","inReplyTo":"alpine.DEB.2.20.1608051728380.5786@virtualbox","subject":"Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-05T21:57:01Z","receivedAt":"2016-08-05T21:57:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Hi Junio & René,\n>\n> On Thu, 4 Aug 2016, Junio C Hamano wrote:\n>\n>> Let's try it this way. How about this as a replacement?\n>\n> I like it (with the if (s2) test intead of if (s1), of course). But please\n> record René as author, maybe mentioning myself with a \"Diagnosed-by:\"\n> line.\n\nHmph. I cannot do that unilaterally without waiting for René to\nrespond, though. In any case, with only header and footer changes,\nhere is what will appear in 'pu'.\n\nThanks.\n\n-- >8 --\nFrom: René Scharfe \nDate: Thu, 4 Aug 2016 23:56:54 +0200\nSubject: [PATCH] nedmalloc: work around overzealous GCC 6 warning\n\nWith GCC 6, the strdup() function is declared with the \"nonnull\"\nattribute, stating that it is not allowed to pass a NULL value as\nparameter.\n\nIn nedmalloc()'s reimplementation of strdup(), Postel's Law is heeded\nand NULL parameters are handled gracefully. GCC 6 complains about that\nnow because it thinks that NULL cannot be passed to strdup() anyway.\n\nBecause the callers in this project of strdup() must be prepared to\ncall any implementation of strdup() supplied by the platform, so it\nis pointless to pretend that it is OK to call it with NULL.\n\nRemove the conditional based on NULL-ness of the input; this\nsquelches the warning.\n\nSee https://gcc.gnu.org/gcc-6/porting_to.html for details.\n\nDiagnosed-by: Johannes Schindelin \nSigned-off-by: René Scharfe \nSigned-off-by: Junio C Hamano \n---\n compat/nedmalloc/nedmalloc.c | 9 ++++-----\n 1 file changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c\nindex 677d1b2..2d4ef59 100644\n--- a/compat/nedmalloc/nedmalloc.c\n+++ b/compat/nedmalloc/nedmalloc.c\n@@ -955,12 +955,11 @@ void **nedpindependent_comalloc(nedpool *p, size_t elems, size_t *sizes, void **\n */\n char *strdup(const char *s1)\n {\n-\tchar *s2 = 0;\n-\tif (s1) {\n-\t\tsize_t len = strlen(s1) + 1;\n-\t\ts2 = malloc(len);\n+\tsize_t len = strlen(s1) + 1;\n+\tchar *s2 = malloc(len);\n+\n+\tif (s2)\n \t\tmemcpy(s2, s1, len);\n-\t}\n \treturn s2;\n }\n #endif\n-- \n2.9.2-766-gd7972a8\n\n"},{"id":"293299","messageId":"57A5136E.20802@web.de","threadId":"43006","inReplyTo":"xmqqk2fuvpzm.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2016-08-05T22:30:06Z","receivedAt":"2016-08-06T20:58:43Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 05.08.2016 um 23:57 schrieb Junio C Hamano:\n> Johannes Schindelin writes:\n>\n>> Hi Junio & René,\n>>\n>> On Thu, 4 Aug 2016, Junio C Hamano wrote:\n>>\n>>> Let's try it this way. How about this as a replacement?\n>>\n>> I like it (with the if (s2) test intead of if (s1), of course). But please\n>> record René as author, maybe mentioning myself with a \"Diagnosed-by:\"\n>> line.\n>\n> Hmph. I cannot do that unilaterally without waiting for René to\n> respond, though. In any case, with only header and footer changes,\n> here is what will appear in 'pu'.\n\nIt's fine with me, thanks. :) Minor comments below.\n\n> Thanks.\n>\n> -- >8 --\n> From: René Scharfe \n> Date: Thu, 4 Aug 2016 23:56:54 +0200\n> Subject: [PATCH] nedmalloc: work around overzealous GCC 6 warning\n>\n> With GCC 6, the strdup() function is declared with the \"nonnull\"\n> attribute, stating that it is not allowed to pass a NULL value as\n> parameter.\n>\n> In nedmalloc()'s reimplementation of strdup(), Postel's Law is heeded\n> and NULL parameters are handled gracefully. GCC 6 complains about that\n> now because it thinks that NULL cannot be passed to strdup() anyway.\n>\n> Because the callers in this project of strdup() must be prepared to\n> call any implementation of strdup() supplied by the platform, so it\n> is pointless to pretend that it is OK to call it with NULL.\n>\n> Remove the conditional based on NULL-ness of the input; this\n> squelches the warning.\n\nMy commit message would have been much shorter, probably too short. But \nperhaps add here: \"Check the return value of malloc() instead to make \nsure we actually got memory to write to.\"\n\n> See https://gcc.gnu.org/gcc-6/porting_to.html for details.\n>\n> Diagnosed-by: Johannes Schindelin \n> Signed-off-by: René Scharfe \n> Signed-off-by: Junio C Hamano \n> ---\n> compat/nedmalloc/nedmalloc.c | 9 ++++-----\n> 1 file changed, 4 insertions(+), 5 deletions(-)\n>\n> diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c\n> index 677d1b2..2d4ef59 100644\n> --- a/compat/nedmalloc/nedmalloc.c\n> +++ b/compat/nedmalloc/nedmalloc.c\n> @@ -955,12 +955,11 @@ void **nedpindependent_comalloc(nedpool *p, size_t elems, size_t *sizes, void **\n> */\n> char *strdup(const char *s1)\n> {\n> -\tchar *s2 = 0;\n> -\tif (s1) {\n> -\t\tsize_t len = strlen(s1) + 1;\n> -\t\ts2 = malloc(len);\n> +\tsize_t len = strlen(s1) + 1;\n> +\tchar *s2 = malloc(len);\n> +\n\nYou added this blank line. OK.\n\n> +\tif (s2)\n> \t\tmemcpy(s2, s1, len);\n> -\t}\n> \treturn s2;\n> }\n> #endif\n>\n"},{"id":"293304","messageId":"alpine.DEB.2.20.1608061014170.5786@virtualbox","threadId":"43006","inReplyTo":"xmqqpopnxkfv.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-06T08:19:52Z","receivedAt":"2016-08-06T21:59:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 5 Aug 2016, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > This setup will from now on test next & pu in the Git for Windows SDK, and\n> > rebase Git for Windows' current master to git.git's maint, master, next &\n> > pu, every morning after a weekday (unless I forget to turn on my laptop,\n> > that is).\n> \n> Sounds really good. Thanks.\n\nReport for today:\n\n- `pu` and `next` pass\n\n- rebasing the Windows-specific patches failed because of the merge\n conflicts when trying to apply the GCC-6 pragma, which I have on Git for\n Windows' `master` already.\n\nNote: I fixed said merge conflicts locally, and the way I set this up, the\nCI job learns the conflict resolution and will apply it automatically next\ntime it runs.\n\nSo: all greeeeen today.\n\nCiao,\nDscho\n"}]}