Re: [PATCH] git-compat-util: avoid redefining system function names
- From
Jeff King <peff@peff.net>
- Date
- Dec 2, 2022, 11:05 UTC
- Message-ID
- <Y4ncAhIqHkckMljb@coredump.intra.peff.net>
- In-Reply-To
- <Y4nN2h4FIYGNjCSI@debian.me>
On Fri, Dec 02, 2022 at 05:05:14PM +0700, Bagas Sanjaya wrote:
Show 13 quoted lines
> 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