From: René Scharfe <l.s.r@web.de>
Date: Fri, 05 Aug 2016 22:30:06 GMT
Subject: Re: [PATCH 2/2] nedmalloc: work around overzealous GCC 6 warning
Message-ID: <57A5136E.20802@web.de>
In-Reply-To: <xmqqk2fuvpzm.fsf@gitster.mtv.corp.google.com>

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
>