Re: [PATCH v2 2/2] grep: stop looking at random places for .gitattributes

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 17, 2012, 07:33 UTC
Message-ID: <7vk3uppn7r.fsf@alter.siamese.dyndns.org>
In-Reply-To: <507E58CF.2040803@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

Show 12 quoted lines

> diff --git a/compat/mingw.c b/compat/mingw.c
> index afc892d..4e63838 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -335,6 +335,28 @@ FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream)
>  	return freopen(filename, otype, stream);
>  }
>  
> +#undef fflush
> +int mingw_fflush(FILE *stream)
> +{
> +	int ret = fflush(stream);

The "#undef" above is a bit unfortunate.

Whenever I see this construct I start to wonder "I know this is to disable our own #define we have elsewhere that renames fflush() to mingw_fflush(), but what happens if the system include implements fflush() as a macro?"

A better organization might be

 - make "int mingw_fflush(FILE *);" declaration available to all the
   callers and to this part of the file; and

 - make "#define fflush(x) mingw_fflush(x)" macro visible when
   compiling the rest of the system, but make it invisible to the
   implementation of the emulation function.

The latter implies that a function in the emulation layer, if it needs to fflush(), would explicitly call mingw_fflush().

I know you did this knowing that it is not an issue on your platform, and this file is only used on your platform anyway, so I do not think we should address such a reorganization right now, but it is something we may want to keep an eye on, as other people may later try to stub away a real macro imitating this part of the code.

Thanks for following through.

Sometimes discussions on our list result in participant feeling satisified with the conclusion without completing the last mile of producing and applying the patch, which I find only after a few month when I'm trawling the list archive for anything we missed.

Now I'll have to do my part and queue this to my tree ;-)