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

Am 03.09.21 um 22:13 schrieb Carlo Marcelo Arenas Belón:

On Fri, Sep 03, 2021 at 08:47:02PM +0200, René Scharfe wrote:

quoted

Am 03.09.21 um 19:02 schrieb Carlo Marcelo Arenas Belón:

quoted

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.

quoted

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

True, other pointers could be smaller on some machines.

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

quoted

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

so something like this on top?

Nah, I like your original version better now that I understand the warning..

Though for upstream it would make more sense to report the caller-supplied
pointer value in the error message than a casted one..