Re: [PATCH v2] git-compat-util: undefine fileno if defined
Dan McGregor <dkm560@usask.ca> writes:
Show 16 quoted lines
>>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.
>
> Yes, this looks pretty reasonable to me too.
Sorry for pinging this ancient thread, but while reviewing the stalled topics, this one caught my attention. The very original <20190201193004.88736-1-dan.mcgregor@usask.ca> said that the problem it wants to solve was that the code that passes (void*) parameter to fileno(), fflush() and rewind() misbehaved, as these are all macros on your system.
We solved the problem for fileno() being a macro eventually with 18a4f6be ("git-compat-util: work around fileno(fp) that is a macro", 2019-02-12), but what about the other two?
Here comes a weather-balloon to see if we should pursue tying this loose end.
One thing to note is that this reveals that the build rule for vcs-svn stuff may be cleaned up before we do this as a separate step, but I think I saw another topics to move it out of the main build so perhaps I'll leave it out and leave it to be worried about separately ;-)
-- >8 --
From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 8 May 2019 16:12:20 +0900
Subject: [PATCH] git-compat-util: rewind(fp)/fflush(fp) could be macros
On various BSD's, rewind(fp) is implemented as a macro that directly accesses the fields in the FILE * object, which breaks a function that accepts a "void *fp" parameter and calls rewind(fp) and expect it to work. The same issue has been resolved for fileno(fp) earlier, which is shared by fflush(fp) as well.
Work it around by adding a compile-time knob REWIND_IS_A_MACRO and FFLUSH_IS_A_MACRO for these two other functions that tells us to insert a real helper function in the middle of the callchain.
I'll leave it up to those who do work on affected platforms to add entries to config.mak.uname.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Makefile | 18 ++++++++++++++++--
compat/fflush.c | 7 +++++++
compat/rewind.c | 7 +++++++
git-compat-util.h | 16 ++++++++++++++++
4 files changed, 46 insertions(+), 2 deletions(-)
create mode 100644 compat/fflush.c
create mode 100644 compat/rewind.c
Show changes to 4 files +46 −2
Makefile, compat/fflush.c, compat/rewind.c, git-compat-util.h
diff --git a/Makefile b/Makefile
index 6e8d017e8e..1e18092583 100644
--- a/Makefile
+++ b/Makefile
@@ -433,6 +433,10 @@ all::
#
# Define HAVE_GETDELIM if your system has the getdelim() function.
#
+# Define FFLUSH_IS_A_MACRO if fflush() is a macro, not a real function.
+#
+# Define REWIND_IS_A_MACRO if rewind() 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.
#
@@ -1795,6 +1799,16 @@ ifdef HAVE_WPGMPTR
BASIC_CFLAGS += -DHAVE_WPGMPTR
endif
+ifdef FFLUSH_IS_A_MACRO
+ COMPAT_CFLAGS += -DFFLUSH_IS_A_MACRO
+ COMPAT_OBJS += compat/fflush.o
+endif
+
+ifdef REWIND_IS_A_MACRO
+ COMPAT_CFLAGS += -DREWIND_IS_A_MACRO
+ COMPAT_OBJS += compat/rewind.o
+endif
+
ifeq ($(TCLTK_PATH),)
NO_TCLTK = NoThanks
endif
@@ -2404,8 +2418,8 @@ git-http-push$X: http.o http-push.o GIT-LDFLAGS $(GITLIBS)
$(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS)
git-remote-testsvn$X: remote-testsvn.o GIT-LDFLAGS $(GITLIBS) $(VCSSVN_LIB)
- $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) \
- $(VCSSVN_LIB)
+ $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
+ $(VCSSVN_LIB) $(LIBS)
$(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY)
$(QUIET_LNCP)$(RM) $@ && \
diff --git a/compat/fflush.c b/compat/fflush.c
new file mode 100644
index 0000000000..55ac3e5542
--- /dev/null
+++ b/compat/fflush.c
@@ -0,0 +1,7 @@
+#define COMPAT_CODE_FFLUSH
+#include "../git-compat-util.h"
+
+int git_fflush(FILE *stream)
+{
+ return fflush(stream);
+}
diff --git a/compat/rewind.c b/compat/rewind.c
new file mode 100644
index 0000000000..e56656ff96
--- /dev/null
+++ b/compat/rewind.c
@@ -0,0 +1,7 @@
+#define COMPAT_CODE_REWIND
+#include "../git-compat-util.h"
+
+void git_rewind(FILE *stream)
+{
+ rewind(stream);
+}
diff --git a/git-compat-util.h b/git-compat-util.h
index 29a19902aa..f35a857785 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -1234,6 +1234,22 @@ struct tm *git_gmtime_r(const time_t *, struct tm *);
#define getc_unlocked(fh) getc(fh)
#endif
+#ifdef FFLUSH_IS_A_MACRO
+int git_fflush(FILE *stream);
+# ifndef COMPAT_CODE_FFLUSH
+# undef fflush
+# define fflush(p) git_fflush(p)
+# endif
+#endif
+
+#ifdef REWIND_IS_A_MACRO
+void git_rewind(FILE *stream);
+# ifndef COMPAT_CODE_REWIND
+# undef rewind
+# define rewind(p) git_rewind(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
--
2.21.0-777-g83232e3864
