Re: [msysGit] [PATCH] mingw: redefine the wrapper macro after the corresponding function

From
Johannes Sixt <hidden>
Date
2016-06-15 23:01:29
Reply to
Stepan Kasal, 2016-06-15
Am 05.06.2014 10:05, schrieb Stepan Kasal:
quoted hunk ↗ jump to hunk
mingw.c defines several wrapper functionsi, like mingw_unlink().
These wrappers are deployed by macros like this:
	#define unlink mingw_unlink
The function itself is preceded by #undef, leaving the wrapper out
of the game for the rest of mingw.c.

This was not probably intentional; for example, there are three
calls to open() below the definition mingw_open() that probably
have no reason to circumvent the wrapper.
OTOH, there is one call to gethostbyname() before it was undefined;
probably happy that it actually calls mingw_gethostbyname().

This patch adds back the #define after each wrapper definition.

Signed-off-by: Stepan Kasal <redacted>
---
 compat/mingw.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
diff --git a/compat/mingw.c b/compat/mingw.c
index a0e13bc..e7193c0 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -224,6 +224,7 @@ int mingw_unlink(const char *pathname)
 	       ret = unlink(pathname);
 	return ret;
 }
+#define unlink mingw_unlink
(etc...)

I don't particularly like this approach: It robs the precise control of
which function we can invoke from other places in mingw.c.

Within mingw.c, if some other function inside mingw.c wants to use
mingw_unlink, then it should be written as 'mingw_unlink(foo)', not
'unlink(foo)'.

So, IMO the macros should be #undef'ed at the top of the file, and all
users (like the open() and gethostbyname() invocations that you
identified) should be audited and changed to call the function they
actually need (i.e., the system open vs. mingw_open).

-- Hannes
Thread (11 messages) 11 messages, 3 authors, last reply 2016-06-15
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help