All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Patches to let Git build with GCC 6 and DEVELOPER=SureWhyNot
@ 2016-08-04 16:07 Johannes Schindelin
  2016-08-04 16:07 ` [PATCH 1/2] nedmalloc: fix misleading indentation Johannes Schindelin
  2016-08-04 16:07 ` [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning Johannes Schindelin
  0 siblings, 2 replies; 17+ messages in thread
From: Johannes Schindelin @ 2016-08-04 16:07 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

While working on some local setup to allow my poor little laptop to
build and test the Windows-specific patches of Git for Windows on top of
the upstream branches, my development environment updated to use GCC 6,
and these patches were required.


Johannes Schindelin (2):
  nedmalloc: fix misleading indentation
  nedmalloc: work around overzealous GCC 6 warning

 compat/nedmalloc/nedmalloc.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Published-As: https://github.com/dscho/git/releases/tag/gcc-6-v1
Fetch-It-Via: git fetch https://github.com/dscho/git gcc-6-v1
-- 
2.9.0.281.g286a8d9

base-commit: 80460f513ebd7851953f5402dd9744236128b240

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/2] nedmalloc: fix misleading indentation
  2016-08-04 16:07 [PATCH 0/2] Patches to let Git build with GCC 6 and DEVELOPER=SureWhyNot Johannes Schindelin
@ 2016-08-04 16:07 ` Johannes Schindelin
  2016-08-04 16:07 ` [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning Johannes Schindelin
  1 sibling, 0 replies; 17+ messages in thread
From: Johannes Schindelin @ 2016-08-04 16:07 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

Some code in nedmalloc is indented in a funny way that could be
misinterpreted as if a line after a for loop was included in the loop
body, when it is not.

GCC 6 complains about this in DEVELOPER=YepSure mode.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/nedmalloc/nedmalloc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c
index a0a16eb..677d1b2 100644
--- a/compat/nedmalloc/nedmalloc.c
+++ b/compat/nedmalloc/nedmalloc.c
@@ -938,10 +938,10 @@ void **nedpindependent_comalloc(nedpool *p, size_t elems, size_t *sizes, void **
 	void **ret;
 	threadcache *tc;
 	int mymspace;
-    size_t i, *adjustedsizes=(size_t *) alloca(elems*sizeof(size_t));
-    if(!adjustedsizes) return 0;
-    for(i=0; i<elems; i++)
-	adjustedsizes[i]=sizes[i]<sizeof(threadcacheblk) ? sizeof(threadcacheblk) : sizes[i];
+	size_t i, *adjustedsizes=(size_t *) alloca(elems*sizeof(size_t));
+	if(!adjustedsizes) return 0;
+	for(i=0; i<elems; i++)
+		adjustedsizes[i]=sizes[i]<sizeof(threadcacheblk) ? sizeof(threadcacheblk) : sizes[i];
 	GetThreadCache(&p, &tc, &mymspace, 0);
 	GETMSPACE(m, p, tc, mymspace, 0,
 	      ret=mspace_independent_comalloc(m, elems, adjustedsizes, chunks));
-- 
2.9.0.281.g286a8d9



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning
  2016-08-04 16:07 [PATCH 0/2] Patches to let Git build with GCC 6 and DEVELOPER=SureWhyNot Johannes Schindelin
  2016-08-04 16:07 ` [PATCH 1/2] nedmalloc: fix misleading indentation Johannes Schindelin
@ 2016-08-04 16:07 ` Johannes Schindelin
  2016-08-04 17:41   ` Junio C Hamano
  2016-08-04 21:56   ` René Scharfe
  1 sibling, 2 replies; 17+ messages in thread
From: Johannes Schindelin @ 2016-08-04 16:07 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

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.

Let's just shut up GCC >= 6 in that case and go on with our lives.

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

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/nedmalloc/nedmalloc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c
index 677d1b2..3f28c0b 100644
--- a/compat/nedmalloc/nedmalloc.c
+++ b/compat/nedmalloc/nedmalloc.c
@@ -956,6 +956,9 @@ void **nedpindependent_comalloc(nedpool *p, size_t elems, size_t *sizes, void **
 char *strdup(const char *s1)
 {
 	char *s2 = 0;
+#if __GNUC__ >= 6
+#pragma GCC diagnostic ignored "-Wnonnull-compare"
+#endif
 	if (s1) {
 		size_t len = strlen(s1) + 1;
 		s2 = malloc(len);
-- 
2.9.0.281.g286a8d9

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning
  2016-08-04 16:07 ` [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning Johannes Schindelin
@ 2016-08-04 17:41   ` Junio C Hamano
  2016-08-05 15:38     ` Johannes Schindelin
  2016-08-04 21:56   ` René Scharfe
  1 sibling, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2016-08-04 17:41 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> 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.
>
> Let's just shut up GCC >= 6 in that case and go on with our lives.
>
> See https://gcc.gnu.org/gcc-6/porting_to.html for details.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  compat/nedmalloc/nedmalloc.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c
> index 677d1b2..3f28c0b 100644
> --- a/compat/nedmalloc/nedmalloc.c
> +++ b/compat/nedmalloc/nedmalloc.c
> @@ -956,6 +956,9 @@ void **nedpindependent_comalloc(nedpool *p, size_t elems, size_t *sizes, void **
>  char *strdup(const char *s1)
>  {
>  	char *s2 = 0;
> +#if __GNUC__ >= 6
> +#pragma GCC diagnostic ignored "-Wnonnull-compare"
> +#endif
>  	if (s1) {
>  		size_t len = strlen(s1) + 1;
>  		s2 = malloc(len);

Is it a common convention to place "#pragma GCC diagnostic"
immediately before the statement you want to affect, and have the
same pragma in effect until the end of the compilation unit?

I know this function is at the end and it is not worth doing
push/ignored/pop dance, and I assumed that it is the reason why we
see a single "ignore from here on", which is much simpler, but it is
somewhat distracting.  It made me wonder if it makes it easier to
read and less distracting to have these three lines in front of and
outside the function definition, while thinking that it would have a
documentation value to have it immediately before the statement you
want to affect.  Help me convince myself that this is the best
place.

Thanks.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning
  2016-08-04 16:07 ` [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning Johannes Schindelin
  2016-08-04 17:41   ` Junio C Hamano
@ 2016-08-04 21:56   ` René Scharfe
  2016-08-04 22:33     ` Junio C Hamano
  2016-08-04 22:39     ` Junio C Hamano
  1 sibling, 2 replies; 17+ messages in thread
From: René Scharfe @ 2016-08-04 21:56 UTC (permalink / raw)
  To: Johannes Schindelin, git; +Cc: Junio C Hamano

Am 04.08.2016 um 18:07 schrieb Johannes Schindelin:
> 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.
>
> Let's just shut up GCC >= 6 in that case and go on with our lives.

This version of strdup() is only compiled if nedmalloc is used instead
of the system allocator.  That means we can't rely on strdup() being
able to take NULL -- some (most?) platforms won't like it.  Removing
the NULL check would be a more general and overall easier way out, no?

But it should check the result of malloc() before copying.
---
  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 a0a16eb..cc18f0c 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;
+	char *s2 = malloc(len);
+	if (s2)
  		memcpy(s2, s1, len);
-	}
  	return s2;
  }
  #endif
-- 
2.9.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning
  2016-08-04 21:56   ` René Scharfe
@ 2016-08-04 22:33     ` Junio C Hamano
  2016-08-04 22:39     ` Junio C Hamano
  1 sibling, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2016-08-04 22:33 UTC (permalink / raw)
  To: René Scharfe; +Cc: Johannes Schindelin, git

René Scharfe <l.s.r@web.de> writes:

> This version of strdup() is only compiled if nedmalloc is used instead
> of the system allocator.  That means we can't rely on strdup() being
> able to take NULL -- some (most?) platforms won't like it.  Removing
> the NULL check would be a more general and overall easier way out, no?

The callers of this version must be prepared to call a version of
strdup() that does not accept NULL, so in a sense, passing NULL to
this function is already an error in the context of this project.

That sounds like a good rationale to take this more straight-forward
approach.

> But it should check the result of malloc() before copying.
> ---
>  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 a0a16eb..cc18f0c 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;
> +	char *s2 = malloc(len);
> +	if (s2)
>  		memcpy(s2, s1, len);
> -	}
>  	return s2;
>  }
>  #endif

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning
  2016-08-04 21:56   ` René Scharfe
  2016-08-04 22:33     ` Junio C Hamano
@ 2016-08-04 22:39     ` Junio C Hamano
  2016-08-05  5:36       ` Johannes Sixt
  2016-08-05 15:34       ` Johannes Schindelin
  1 sibling, 2 replies; 17+ messages in thread
From: Junio C Hamano @ 2016-08-04 22:39 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: René Scharfe, git

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

-- >8 --
From: Johannes Schindelin <johannes.schindelin@gmx.de>
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 <johannes.schindelin@gmx.de>
Helped-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning
  2016-08-04 22:39     ` Junio C Hamano
@ 2016-08-05  5:36       ` Johannes Sixt
  2016-08-05  5:40         ` Johannes Sixt
  2016-08-05 15:34       ` Johannes Schindelin
  1 sibling, 1 reply; 17+ messages in thread
From: Johannes Sixt @ 2016-08-05  5:36 UTC (permalink / raw)
  To: Junio C Hamano, Johannes Schindelin; +Cc: René Scharfe, git

Am 05.08.2016 um 00:39 schrieb Junio C Hamano:
> @@ -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...

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

-- Hannes


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning
  2016-08-05  5:36       ` Johannes Sixt
@ 2016-08-05  5:40         ` Johannes Sixt
  2016-08-05  6:24           ` Junio C Hamano
  0 siblings, 1 reply; 17+ messages in thread
From: Johannes Sixt @ 2016-08-05  5:40 UTC (permalink / raw)
  To: Junio C Hamano, Johannes Schindelin; +Cc: René Scharfe, git

Am 05.08.2016 um 07:36 schrieb Johannes Sixt:
> Am 05.08.2016 um 00:39 schrieb Junio C Hamano:
>> @@ -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.

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

-- Hannes


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning
  2016-08-05  5:40         ` Johannes Sixt
@ 2016-08-05  6:24           ` Junio C Hamano
  2016-08-05  7:42             ` Jeff King
  0 siblings, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2016-08-05  6:24 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Johannes Schindelin, René Scharfe, git

Johannes Sixt <j6t@kdbg.org> writes:

> Oh! This is a typo. You meant to check s2 for NULL.
>
> And the declaration for s2 should remain, of course.

Yeah, the moral of the story is don't try to do something you do not
usually do X-<.

The second try (the log message is the same).

I do not know if we want to worry about st_add(1, strlen(s1))
overflow around here, though.

 compat/nedmalloc/nedmalloc.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c
index 677d1b2..2d4ef59 100644
--- a/compat/nedmalloc/nedmalloc.c
+++ b/compat/nedmalloc/nedmalloc.c
@@ -955,12 +955,11 @@ 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;
+	char *s2 = malloc(len);
+
+	if (s2)
 		memcpy(s2, s1, len);
-	}
 	return s2;
 }
 #endif

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning
  2016-08-05  6:24           ` Junio C Hamano
@ 2016-08-05  7:42             ` Jeff King
  0 siblings, 0 replies; 17+ messages in thread
From: Jeff King @ 2016-08-05  7:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Sixt, Johannes Schindelin, René Scharfe, git

On Thu, Aug 04, 2016 at 11:24:32PM -0700, Junio C Hamano wrote:

> I do not know if we want to worry about st_add(1, strlen(s1))
> overflow around here, though.
> [...]
> +	size_t len = strlen(s1) + 1;

I wondered that, too, but I don't think it's possible.

To overflow the size_t with "+1", strlen() must return the maximum value
that it can hold. But such a string would need one more byte than that,
for its trailing NUL. So assuming you cannot have a string that exceeds
size_t in the first place, I think it is impossible to overflow here.

-Peff

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning
  2016-08-04 22:39     ` Junio C Hamano
  2016-08-05  5:36       ` Johannes Sixt
@ 2016-08-05 15:34       ` Johannes Schindelin
  2016-08-05 16:13         ` Junio C Hamano
  2016-08-05 21:57         ` Junio C Hamano
  1 sibling, 2 replies; 17+ messages in thread
From: Johannes Schindelin @ 2016-08-05 15:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: René Scharfe, git

[-- Attachment #1: Type: text/plain, Size: 745 bytes --]

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning
  2016-08-04 17:41   ` Junio C Hamano
@ 2016-08-05 15:38     ` Johannes Schindelin
  0 siblings, 0 replies; 17+ messages in thread
From: Johannes Schindelin @ 2016-08-05 15:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 1916 bytes --]

Hi Junio,

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

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > 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.
> >
> > Let's just shut up GCC >= 6 in that case and go on with our lives.
> >
> > See https://gcc.gnu.org/gcc-6/porting_to.html for details.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  compat/nedmalloc/nedmalloc.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c
> > index 677d1b2..3f28c0b 100644
> > --- a/compat/nedmalloc/nedmalloc.c
> > +++ b/compat/nedmalloc/nedmalloc.c
> > @@ -956,6 +956,9 @@ void **nedpindependent_comalloc(nedpool *p, size_t elems, size_t *sizes, void **
> >  char *strdup(const char *s1)
> >  {
> >  	char *s2 = 0;
> > +#if __GNUC__ >= 6
> > +#pragma GCC diagnostic ignored "-Wnonnull-compare"
> > +#endif
> >  	if (s1) {
> >  		size_t len = strlen(s1) + 1;
> >  		s2 = malloc(len);
> 
> Is it a common convention to place "#pragma GCC diagnostic"
> immediately before the statement you want to affect, and have the
> same pragma in effect until the end of the compilation unit?

Uh oh. This was a brain fart. I somehow confused the way pragmas work with
the way __attribute__s work. You are correct, of course, that the pragma
affects the entire remainder of the file, not just this statement.

Luckily, René came up with a much more elegant solution, so that Git's
history does not have to shame me eternally.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning
  2016-08-05 15:34       ` Johannes Schindelin
@ 2016-08-05 16:13         ` Junio C Hamano
  2016-08-06  8:19           ` Johannes Schindelin
  2016-08-05 21:57         ` Junio C Hamano
  1 sibling, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2016-08-05 16:13 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: René Scharfe, git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> 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).

Sounds really good.  Thanks.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning
  2016-08-05 15:34       ` Johannes Schindelin
  2016-08-05 16:13         ` Junio C Hamano
@ 2016-08-05 21:57         ` Junio C Hamano
  2016-08-05 22:30           ` René Scharfe
  1 sibling, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2016-08-05 21:57 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: René Scharfe, git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> 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.

Hmph.  I cannot do that unilaterally without waiting for René to
respond, though.  In any case, with only header and footer changes,
here is what will appear in 'pu'.

Thanks.

-- >8 --
From: René Scharfe <l.s.r@web.de>
Date: Thu, 4 Aug 2016 23:56:54 +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.

Diagnosed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 compat/nedmalloc/nedmalloc.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c
index 677d1b2..2d4ef59 100644
--- a/compat/nedmalloc/nedmalloc.c
+++ b/compat/nedmalloc/nedmalloc.c
@@ -955,12 +955,11 @@ 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;
+	char *s2 = malloc(len);
+
+	if (s2)
 		memcpy(s2, s1, len);
-	}
 	return s2;
 }
 #endif
-- 
2.9.2-766-gd7972a8


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning
  2016-08-05 21:57         ` Junio C Hamano
@ 2016-08-05 22:30           ` René Scharfe
  0 siblings, 0 replies; 17+ messages in thread
From: René Scharfe @ 2016-08-05 22:30 UTC (permalink / raw)
  To: Junio C Hamano, Johannes Schindelin; +Cc: git

Am 05.08.2016 um 23:57 schrieb Junio C Hamano:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> 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.
>
> Hmph.  I cannot do that unilaterally without waiting for René to
> respond, though.  In any case, with only header and footer changes,
> here is what will appear in 'pu'.

It's fine with me, thanks. :)  Minor comments below.

> Thanks.
>
> -- >8 --
> From: René Scharfe <l.s.r@web.de>
> Date: Thu, 4 Aug 2016 23:56:54 +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.

My commit message would have been much shorter, probably too short.  But 
perhaps add here: "Check the return value of malloc() instead to make 
sure we actually got memory to write to."

> See https://gcc.gnu.org/gcc-6/porting_to.html for details.
>
> Diagnosed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>   compat/nedmalloc/nedmalloc.c | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c
> index 677d1b2..2d4ef59 100644
> --- a/compat/nedmalloc/nedmalloc.c
> +++ b/compat/nedmalloc/nedmalloc.c
> @@ -955,12 +955,11 @@ 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;
> +	char *s2 = malloc(len);
> +

You added this blank line.  OK.

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning
  2016-08-05 16:13         ` Junio C Hamano
@ 2016-08-06  8:19           ` Johannes Schindelin
  0 siblings, 0 replies; 17+ messages in thread
From: Johannes Schindelin @ 2016-08-06  8:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: René Scharfe, git

Hi Junio,

On Fri, 5 Aug 2016, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > 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).
> 
> Sounds really good.  Thanks.

Report for today:

- `pu` and `next` pass

- rebasing the Windows-specific patches failed because of the merge
  conflicts when trying to apply the GCC-6 pragma, which I have on Git for
  Windows' `master` already.

Note: I fixed said merge conflicts locally, and the way I set this up, the
CI job learns the conflict resolution and will apply it automatically next
time it runs.

So: all greeeeen today.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2016-08-06 21:59 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-04 16:07 [PATCH 0/2] Patches to let Git build with GCC 6 and DEVELOPER=SureWhyNot Johannes Schindelin
2016-08-04 16:07 ` [PATCH 1/2] nedmalloc: fix misleading indentation Johannes Schindelin
2016-08-04 16:07 ` [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning Johannes Schindelin
2016-08-04 17:41   ` Junio C Hamano
2016-08-05 15:38     ` Johannes Schindelin
2016-08-04 21:56   ` René Scharfe
2016-08-04 22:33     ` Junio C Hamano
2016-08-04 22:39     ` Junio C Hamano
2016-08-05  5:36       ` Johannes Sixt
2016-08-05  5:40         ` Johannes Sixt
2016-08-05  6:24           ` Junio C Hamano
2016-08-05  7:42             ` Jeff King
2016-08-05 15:34       ` Johannes Schindelin
2016-08-05 16:13         ` Junio C Hamano
2016-08-06  8:19           ` Johannes Schindelin
2016-08-05 21:57         ` Junio C Hamano
2016-08-05 22:30           ` René Scharfe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.