Re: [PATCH v2] git-compat-util: undefine fileno if defined

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

Dan McGregor <dan.mcgregor@usask.ca> writes:

> Commit 8dd2e88a92 ("http: support file handles for HTTP_KEEP_ERROR",
> 2019-01-10) introduced an implicit assumption that rewind, fileno, and
> fflush are functions. At least on FreeBSD fileno is not, and as such
> passing a void * failed.
>
> All systems tested (FreeBSD and NetBSD) that define fineo as a macro

s/fineo/fileno/

> also have a function defined. Undefine the macro on these systems so
> that the function is used.

That smells like the patch is assuming a bit too much.  A platform
you did not inspect may have it as a macro but the implementation
may still be usable without breakage like the one we saw on FreeBSD,
for example.

It also robs us the possibility of overriding fileno() with our own
macro earlier in this file, like we do for many functions you see in
the output from this:

 $ git grep '#define .* git' git-compat-util.h

In general, I'd like to see people thinking twice about an overly
simple approach when they justify it with "this should work
everywhere I know of".

> Signed-off-by: Dan McGregor <dan.mcgregor@usask.ca>
> ---
>  git-compat-util.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 29a19902aa..b5489bbcf2 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -764,6 +764,15 @@ char *gitstrdup(const char *s);
>  extern FILE *git_fopen(const char*, const char*);
>  #endif
>  
> +/* Some systems (the various BSDs for eaxmple) define

Style:

/*
 * Our multi-line comment starts with a lone slash-star
 * and ends with a lone star-slash, like this.
 */

> + * fileno as a macro as an optimization. All systems I
> + * know about also define it as a real funcion, so use
> + * the real function to allow passing void *s to fileno.
> + */
> +#ifdef fileno
> +# undef fileno
> +#endif
> +
>  #ifdef SNPRINTF_RETURNS_BOGUS
>  #ifdef snprintf
>  #undef snprintf