On Fri, Feb 08, 2019 at 08:36:21PM -0600, Dan McGregor wrote:
> 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
OpenBSD or NetBSD? From this [1], it looks like OpenBSD fails while
NetBSD compiles ok (and fails to run some tests)
[1] https://gitlab.com/git-vcs/git-ci/pipelines/47139741/failures
> also have a function defined. Undefine the macro on these systems so
> that the function is used.
I don't think this is enough. At least fbsd defines this
#define fileno(p) (!__isthreaded ? __sfileno(p) : (fileno)(p))
so one of the two functions will be used depending on __isthreaded
flag. Your forcing to use fileno, ignoring __sfileno, is technically
not correct.
For the record, at least fbsd also defines feof, ferror, clearerr,
getc and putc in the same way. But at least I don't see how something
like feof(fp++) could cause bad side effects.
So, how about something like this? A teeny bit longer than your
version, but I think it's easier to control long term.
-- 8< --
diff --git a/Makefile b/Makefile
index 0e13a5b469..44cfc63fc4 100644
--- a/Makefile
+++ b/Makefile
@@ -433,6 +433,8 @@ all::
#
# Define HAVE_GETDELIM if your system has the getdelim() function.
#
+# Define FILENO_IS_A_MACRO is fileno() is a macro, not a real function.
+#
# Define PAGER_ENV to a SP separated VAR=VAL pairs to define
# default environment variables to be passed when a pager is spawned, e.g.
#
@@ -1800,6 +1802,11 @@ ifdef HAVE_WPGMPTR
BASIC_CFLAGS += -DHAVE_WPGMPTR
endif
+ifdef FILENO_IS_A_MACRO
+ COMPAT_CFLAGS += -DFILENO_IS_A_MACRO
+ COMPAT_OBJS += compat/fileno.o
+endif
+
ifeq ($(TCLTK_PATH),)
NO_TCLTK = NoThanks
endif
diff --git a/compat/fileno.c b/compat/fileno.c
new file mode 100644
index 0000000000..7b105f4cd7
--- /dev/null
+++ b/compat/fileno.c
@@ -0,0 +1,7 @@
+#define COMPAT_CODE
+#include "../git-compat-util.h"
+
+int git_fileno(FILE *stream)
+{
+ return fileno(stream);
+}
diff --git a/config.mak.uname b/config.mak.uname
index 786bb2f913..01f62674a4 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -221,6 +221,7 @@ ifeq ($(uname_S),FreeBSD)
HAVE_BSD_KERN_PROC_SYSCTL = YesPlease
PAGER_ENV = LESS=FRX LV=-c MORE=FRX
FREAD_READS_DIRECTORIES = UnfortunatelyYes
+ FILENO_IS_A_MACRO = UnfortunatelyYes
endif
ifeq ($(uname_S),OpenBSD)
NO_STRCASESTR = YesPlease
@@ -234,6 +235,7 @@ ifeq ($(uname_S),OpenBSD)
HAVE_BSD_KERN_PROC_SYSCTL = YesPlease
PROCFS_EXECUTABLE_PATH = /proc/curproc/file
FREAD_READS_DIRECTORIES = UnfortunatelyYes
+ FILENO_IS_A_MACRO = UnfortunatelyYes
endif
ifeq ($(uname_S),MirBSD)
NO_STRCASESTR = YesPlease
diff --git a/git-compat-util.h b/git-compat-util.h
index 29a19902aa..6573808ebd 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -1234,6 +1234,14 @@ struct tm *git_gmtime_r(const time_t *, struct tm *);
#define getc_unlocked(fh) getc(fh)
#endif
+#ifdef FILENO_IS_A_MACRO
+int git_fileno(FILE *stream);
+# ifndef COMPAT_CODE
+# undef fileno
+# define fileno(p) git_fileno(p)
+# endif
+#endif
+
/*
* Our code often opens a path to an optional file, to work on its
* contents when we can successfully open it. We can ignore a failure
-- 8< --
--
Duy
