Re: [PATCH v3 2/3] win32: allow building with pedantic mode enabled

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 3, 2021, 20:32 UTC
Message-ID: <xmqq7dfxfli5.fsf@gitster.g>
In-Reply-To: <YTKBzi3z5AotirNO@carlos-mbp.lan>

Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:

>> A slightly shorter fix would be to replace "tck" with "mem".  Not as
>> obvious without further context, though.
>
> so something like this on top?

Show 22 quoted lines

> Carlo
> ---- > 8 ----
> diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c
> index edb438a777..14e8c4df4f 100644
> --- a/compat/nedmalloc/nedmalloc.c
> +++ b/compat/nedmalloc/nedmalloc.c
> @@ -510,7 +510,15 @@ static void threadcache_free(nedpool *p, threadcache *tc, int mymspace, void *me
>  	assert(idx<=THREADCACHEMAXBINS);
>  	if(tck==*binsptr)
>  	{
> -		fprintf(stderr, "Attempt to free already freed memory block %p - aborting!\n", (void *)tck);
> +		/*
> +		 * Original code used tck instead of mem, but that was changed
> +		 * to workaround a pedantic warning from mingw64 gcc 10.3 that
> +		 * requires %p to have a explicit (void *) as a parameter.
> +		 *
> +		 * This might seem to be a compiler bug or limitation that
> +		 * should be changed back if fixed for maintanability.
> +		 */
> +		fprintf(stderr, "Attempt to free already freed memory block %p - aborting!\n", mem);
>  		abort();
>  	}

The new comment explains why the original (i.e. unadorned 'tck'), which should work fine, needs to be changed. The reason is because a version of compiler wants an explict (void *) cast to go with the placeholder "%p".

Given that, it would be much better to pass (void *)tck instead of mem, no? Especially since the comment does not say tck and mem have the same pointer value.

Having said lal that, I have to wonder if how much help the developer who is hunting for allocation bug is getting out of a raw pointer value in this message, though.