Re: [PATCH] git-compat-util: avoid redefining system function names

[Jeff King <peff@peff.net>]

On Fri, Dec 02, 2022 at 05:05:14PM +0700, Bagas Sanjaya wrote:

> I got many of redefinition warnings when cross-compiling on Buildroot
> with the patch above, like:
> 
> In file included from cache.h:4,
>                  from common-main.c:1:
> git-compat-util.h:1485: warning: "getc_unlocked" redefined
>  1485 | #define getc_unlocked(fh) getc(fh)
>       | 
> In file included from git-compat-util.h:216,
>                  from cache.h:4,
>                  from common-main.c:1:
> /home/bagas/repo/buildroot/output/host/aarch64-buildroot-linux-uclibc/sysroot/usr/include/stdio.h:835: note: this is the location of the previous definition
>   835 | #define getc_unlocked(_fp) __GETC_UNLOCKED(_fp)

I imagine you'd get that without my patch, too, since I didn't touch the
getc_unlocked() line at all. Or maybe it simply didn't get that far
because of the other redeclared functions.

Anyway, we probably want this on top of the other patch.

-- >8 --
Subject: [PATCH] git-compat-util: undefine system names before redeclaring
 them

When we define a macro to point a system function (e.g., flockfile) to
our custom wrapper, we should make sure that the system did not already
define it as a macro. This is rarely a problem, but can cause
compilation failures if both of these are true:

  - we decide to define our own wrapper even though the system provides
    the function; we know this happens at least with uclibc, which may
    declare flockfile, etc, without _POSIX_THREAD_SAFE_FUNCTIONS

  - the system version is declared as a macro; we know this happens at
    least with uclibc's version of getc_unlocked()

So just handling getc_unlocked() would be sufficient to deal with the
real-world case we've seen. But since it's easy to do, we may as well be
defensive about the other macro wrappers added in the previous patch.

Signed-off-by: Jeff King <peff@peff.net>
---
There may be other similar cases lurking throughout the code base, but I
don't think it's worth anybody's time to go looking for them. If one of
them triggers on a real platform, we can deal with it then.

 git-compat-util.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index 83ec7b7941..76e4b11131 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -346,6 +346,7 @@ static inline int git_setitimer(int which UNUSED,
 				struct itimerval *newvalue UNUSED) {
 	return 0; /* pretend success */
 }
+#undef setitimer
 #define setitimer(which,value,ovalue) git_setitimer(which,value,ovalue)
 #endif
 
@@ -1480,6 +1481,9 @@ static inline void git_funlockfile(FILE *fh UNUSED)
 {
 	; /* nothing */
 }
+#undef flockfile
+#undef funlockfile
+#undef getc_unlocked
 #define flockfile(fh) git_flockfile(fh)
 #define funlockfile(fh) git_funlockfile(fh)
 #define getc_unlocked(fh) getc(fh)
-- 
2.39.0.rc1.456.gb53e2f823e