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

["Carlo Marcelo Arenas Belón" <carenas@gmail.com>]

On Fri, Sep 03, 2021 at 08:47:02PM +0200, René Scharfe wrote:
> Am 03.09.21 um 19:02 schrieb Carlo Marcelo Arenas Belón:
> > diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c
> > index 1cc31c3502..edb438a777 100644
> > --- a/compat/nedmalloc/nedmalloc.c
> > +++ b/compat/nedmalloc/nedmalloc.c
> > @@ -510,7 +510,7 @@ 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", tck);
> > +		fprintf(stderr, "Attempt to free already freed memory block %p - aborting!\n", (void *)tck);
> 
> This change is not mentioned in the commit message.

got me there, I was intentionally trying to ignore it since nedmalloc gives
me PTSD and is obsoleted AFAIK[1], so just adding a casting to void (while
ugly) was also less intrusive.

> compat/nedmalloc/nedmalloc.c:513:82: error: format specifies type 'void *' but the argument has type 'threadcacheblk *' (aka 'struct threadcacheblk_t *') [-Werror,-Wformat-pedantic]
>                 fprintf(stderr, "Attempt to free already freed memory block %p - aborting!\n", tck);
>                                                                             ~~                 ^~~
> This makes no sense to me, though: Any pointer can be converted to a
> void pointer without a cast in C.  GCC doesn't require void pointers
> for %p even with -pedantic.

strange, gcc-11 prints the following in MacOS for me:

compat/nedmalloc/nedmalloc.c: In function 'threadcache_free':
compat/nedmalloc/nedmalloc.c:522:78: warning: format '%p' expects argument of type 'void *', but argument 3 has type 'threadcacheblk *' {aka 'struct threadcacheblk_t *'} [-Wformat=]
  522 |                 fprintf(stderr, "Attempt to free already freed memory block %p - aborting!\n", tck);
      |                                                                             ~^                 ~~~
      |                                                                              |                 |
      |                                                                              void *            threadcacheblk * {aka struct threadcacheblk_t *}

I think the rationale is that it is better to be safe than sorry, and since
the parameter is variadic there is no chance for the compiler to do any
implicit type casting (unless one is provided explicitly).

clang 14 does also trigger a warning, so IMHO this code will be needed
until nedmalloc is retired.

> A slightly shorter fix would be to replace "tck" with "mem".  Not as
> obvious without further context, though.

so something like this on top?

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();
 	}
 #ifdef FULLSANITYCHECKS