Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined

On Tue, Sep 17, 2013 at 11:46 PM, Junio C Hamano [off-list ref] wrote:

quoted

I don't think people on other platforms seeing the ugliness is really
an issue. After all, the file is called git-*compat*-util.h;

Well, judging from the way Linus reacted to the patch, I'd have to
disagree.  After all, that argument leads to the position that
nothing is needed in compat/, no?

My feeling is that Linus' reaction was more about that this
work-around is even necessary (and MinGW is buggy) rather than
applying it to git-compat-util.h and not elsewhere.

One ugliness (lack of sane strcasecmp definition whose address can
be taken) specific to mingw is worked around in compat/mingw.h, and
another ugliness that some people may use compilers without include_next
may need help from another configuration in the Makefile to tell it
where the platform string.h resides.  I am not sure why you see it
as a problem.

I just don't like that the ugliness is spreading out and requires a
change to config.mak.uname now, too. Also, I regard the change to
config.mak.uname by itself as ugly, mainly because you would have to
set SYSTEM_STRING_H_HEADER to some path, but that path might differ
from system to system, depending on where MinGW is installed on
Windows.

quoted

I do insist to avoid GCC-ism in C files,...

To that I tend to agree.  Unconditionally killing inlining for any
mingw compilation in compat/mingw.h may be the simplest (albeit it
may be less than optimal) solution.

I tried to put the __NO_INLINE__ stuff in compat/mingw.h but failed,
it involved the need to shuffle includes in git-compat-util.h around
because winsock2.h already seems to include string.h, and I did not
find a working include order. So I came up with the following, do you
like that better?

diff --git a/compat/string_no_inline.h b/compat/string_no_inline.h
new file mode 100644
index 0000000..51eed52
--- /dev/null
+++ b/compat/string_no_inline.h

@@ -0,0 +1,25 @@
+#ifndef STRING_NO_INLINE_H
+#define STRING_NO_INLINE_H
+
+#ifdef __MINGW32__
+#ifdef __NO_INLINE__
+#define __NO_INLINE_ALREADY_DEFINED
+#else
+#define __NO_INLINE__ /* do not inline strcasecmp() */
+#endif
+#endif
+
+#include <string.h>
+#ifdef HAVE_STRINGS_H
+#include <strings.h> /* for strcasecmp() */
+#endif
+
+#ifdef __MINGW32__
+#ifdef __NO_INLINE_ALREADY_DEFINED
+#undef __NO_INLINE_ALREADY_DEFINED
+#else
+#undef __NO_INLINE__
+#endif
+#endif
+
+#endif

diff --git a/git-compat-util.h b/git-compat-util.h
index db564b7..348dd55 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h

@@ -85,6 +85,8 @@
 #define _NETBSD_SOURCE 1
 #define _SGI_SOURCE 1

+#include "compat/string_no_inline.h"
+
 #ifdef WIN32 /* Both MinGW and MSVC */
 #ifndef _WIN32_WINNT
 #define _WIN32_WINNT 0x0502

@@ -101,10 +103,6 @@
 #include <stddef.h>
 #include <stdlib.h>
 #include <stdarg.h>
-#include <string.h>
-#ifdef HAVE_STRINGS_H
-#include <strings.h> /* for strcasecmp() */
-#endif
 #include <errno.h>
 #include <limits.h>
 #ifdef NEEDS_SYS_PARAM_H

-- 
1.8.3.mingw.1.dirty

-- 
Sebastian Schuberth