On Tue, Sep 17, 2013 at 11:46 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: >> 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. >> 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html