Re: [PATCH 10/11] Fix warnings in nedmalloc when compiling with GCC 4.4.0

Hi,

On Mon, 1 Jun 2009, Junio C Hamano wrote:

Steffen Prohaska [off-list ref] writes:

quoted

@@ -2541,7 +2543,7 @@ struct malloc_params {
 static struct malloc_params mparams;
 
 /* Ensure mparams initialized */
-#define ensure_initialization() (mparams.magic != 0 || init_mparams())
+#define ensure_initialization() if (mparams.magic == 0) init_mparams()
 
 #if !ONLY_MSPACES

The code after the patch looks more fragile than the original.  I know
there currently is no code like:

	if (foo())
        	ensure_initialization();
	else
        	warn("oops");

but this change still feels wrong.

I know, but the whole use of ensure_initialization() feels wrong to me, 
and I did _not_ want to change the code too much, lest we end up 
maintaining a proper fork as has happened with libxdiff.

What issue is this patch trying to work around?  Returned value not 
being used?

I forgot what the GCC warning looked like, and have only text-mode access 
to the web right now, so I cannot access any Windows machine and try 
again.

It was a warning, though, that much I remember.

quoted

diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c
index a381a7d..60a4093 100644
--- a/compat/nedmalloc/nedmalloc.c
+++ b/compat/nedmalloc/nedmalloc.c

@@ -34,7 +34,7 @@ DEALINGS IN THE SOFTWARE.
 /*#define FULLSANITYCHECKS*/
 
 #include "nedmalloc.h"
-#if defined(WIN32) && !defined(__MINGW32__)
+#if defined(WIN32)
  #include <malloc.h>
 #endif

Can somebody enlighten me what this hunk is about, and how it helps GCC
4.4?

It helps in that malloc.h is included even if we happen to compile the 
stuff as a MinGW program.  Otherwise necessary function declarations are 
missing.

There are many "#if[n]def __MINGW32__" remaining in the codebase both
inside and outside compat/ area, so it is not that that symbol is somehow
special.  I cannot even tell which one of the following is closer to the
reason behind this change:

 (1) "Because tacking '&& !defined(__MINGW32__)' after defined(WIN32) is
     unnecessary for such and such reasons, it is removed"; or

 (2) "Because tacking '&& !defined(__MINGW32__)' after defined(WIN32) is
     harmful for such and such reasons, it is removed".

The latter.

Puzzled.

Hopefully less so, now.

Ciao,
Dscho