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

[Junio C Hamano <gitster@pobox.com>]

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

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