Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning

Let's try it this way.  How about this as a replacement?

-- >8 --
From: Johannes Schindelin <redacted>
Date: Thu, 4 Aug 2016 18:07:08 +0200
Subject: [PATCH] nedmalloc: work around overzealous GCC 6 warning

With GCC 6, the strdup() function is declared with the "nonnull"
attribute, stating that it is not allowed to pass a NULL value as
parameter.

In nedmalloc()'s reimplementation of strdup(), Postel's Law is heeded
and NULL parameters are handled gracefully. GCC 6 complains about that
now because it thinks that NULL cannot be passed to strdup() anyway.

Because the callers in this project of strdup() must be prepared to
call any implementation of strdup() supplied by the platform, so it
is pointless to pretend that it is OK to call it with NULL.

Remove the conditional based on NULL-ness of the input; this
squelches the warning.

See https://gcc.gnu.org/gcc-6/porting_to.html for details.

Signed-off-by: Johannes Schindelin <redacted>
Helped-by: René Scharfe [off-list ref]
Signed-off-by: Junio C Hamano <redacted>
---
 compat/nedmalloc/nedmalloc.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c
index 677d1b2..88cd78c 100644
--- a/compat/nedmalloc/nedmalloc.c
+++ b/compat/nedmalloc/nedmalloc.c

@@ -955,12 +955,10 @@ void **nedpindependent_comalloc(nedpool *p, size_t elems, size_t *sizes, void **
  */
 char *strdup(const char *s1)
 {
-	char *s2 = 0;
-	if (s1) {
-		size_t len = strlen(s1) + 1;
-		s2 = malloc(len);
+	size_t len = strlen(s1) + 1;
+	s2 = malloc(len);
+	if (s1)
 		memcpy(s2, s1, len);
-	}
 	return s2;
 }
 #endif

-- 
2.9.2-766-gd7972a8

Am 05.08.2016 um 00:39 schrieb Junio C Hamano:

It does not make sense to check s1 for NULL when it was passed to 
strlen() earlier; strlen() does not accept NULL, either...

 		memcpy(s2, s1, len);
-	}
 	return s2;
 }
 #endif

-- Hannes

Am 05.08.2016 um 07:36 schrieb Johannes Sixt:

Am 05.08.2016 um 00:39 schrieb Junio C Hamano:

quoted

@@ -955,12 +955,10 @@ void **nedpindependent_comalloc(nedpool *p,

size_t elems, size_t *sizes, void **
  */
 char *strdup(const char *s1)
 {
-    char *s2 = 0;
-    if (s1) {
-        size_t len = strlen(s1) + 1;
-        s2 = malloc(len);
+    size_t len = strlen(s1) + 1;
+    s2 = malloc(len);
+    if (s1)

It does not make sense to check s1 for NULL when it was passed to
strlen() earlier; strlen() does not accept NULL, either...

Oh! This is a typo. You meant to check s2 for NULL.

And the declaration for s2 should remain, of course.

quoted

         memcpy(s2, s1, len);
-    }
     return s2;
 }
 #endif

-- Hannes

Hi Junio & René,

On Thu, 4 Aug 2016, Junio C Hamano wrote:

Let's try it this way.  How about this as a replacement?

I like it (with the if (s2) test intead of if (s1), of course). But please
record René as author, maybe mentioning myself with a "Diagnosed-by:"
line.

FWIW today's `pu` does pass the test suite without problems in my CI
setup.

This setup will from now on test next & pu in the Git for Windows SDK, and
rebase Git for Windows' current master to git.git's maint, master, next &
pu, every morning after a weekday (unless I forget to turn on my laptop,
that is).

Once it stabilizes, I will figure out a way how to publish the logs,
because playing Lt Tawney Madison gets old real quick.

Thanks,
Dscho