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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 2, 2009, 15:50 UTC
Message-ID: <7viqjefxcv.fsf@alter.siamese.dyndns.org>
In-Reply-To: <alpine.DEB.1.00.0906021448300.4440@intel-tinevez-2-302>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 14 quoted lines

> Okay, how about this in the commit body in addition?
>
> -- snip --
> Nedmalloc's source code has a cute #define construct to avoid inserting
> an if() statement, because that might interact badly with enclosing if()
> statements.  However, there is no danger of that now, but GCC > 4
> complains "warning: value computed is not used".
>
> Also, with the version of MinGW's headers in msysGit, we need to include 
> malloc.h lest the compiler complain about an "incompatible implicit 
> declaration of built-in function 'alloca'".
> -- snap --
>
> Hmm?

As to the first one, I think your problem description is clear (the problem is clear without being explained to begin with) but "there is no danger of that now" is somewhere between a lame excuse for not doing it the right way and a bug waiting to happen. If "value computed is not used" is the issue, why doesn't this work?

    #define ensure_initialization() (void)(mparams.magic != 0 || init_mparams())

or at least wrap your fragile "if" in a bog-standard do {} while(0), like...

    #define ensure_initialization() \
    	do { if (mparams.magic == 0) init_mparams(); } while (0)

Then you do not have to make excuses, and instead can just say

	Squelch GCC > 4's "value computed not used" warnings from
	ensure_initialization() macro definition.

As to the second one, I think you lost "even if we happen to" part (that logically corresponds to "... that is why we do not want to limit the inclusion to !__MINGW32__ case") from the description, making it less readable...