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

Sebastian Schuberth [off-list ref] writes:

On Fri, Sep 13, 2013 at 10:01 PM, Junio C Hamano [off-list ref] wrote:

quoted

quoted

I don't like the idea of introducing a compat/mingw/string.h because
of two reasons: You would have to add a conditional to include that
string.h instead of the system one anyway,

With -Icompat/mingw passed to the compiler, which is a bog-standard
technique we already use to supply headers the system forgot to
supply or override buggy headers the system is shipped with, you do
not have to change any "#include <string.h>".

Am I mistaken?

Ah, that would work I guess, but you'd still need the include_next.

You can explicitly include the system header from your compatibility
layer, i.e. 

	=== compat/mingw/string.h ===

	#define __NO_INLINE__

	#ifdef SYSTEM_STRING_H_HEADER
        #include SYSTEM_STRING_H_HEADER
        #else
        #include_next <string.h>
	#endif

and then in config.mak.uname, do something like this:

	ifneq (,$(findstring MINGW,$(uname_S)))
	ifndef SYSTEM_STRING_H_HEADER
	SYSTEM_STRING_H_HEADER = "C:\\llvm\include\string.h"
        endif

	COMPAT_CFLAGS += -DSYSTEM_STRING_H_HEADER=$(SYSTEM_STRING_H_HEADER)
	endif

People who have the system header file at different paths can
further override SYSTEM_STRING_H_HEADER in their config.mak.

That would help compilers targetting mingw that do not support
"#include_next" without spreading the damage to other people's
systems, I think.

On Sat, Sep 14, 2013 at 12:06 AM, Junio C Hamano [off-list ref] wrote:

You can explicitly include the system header from your compatibility
layer, i.e.

        === compat/mingw/string.h ===

        #define __NO_INLINE__

        #ifdef SYSTEM_STRING_H_HEADER
        #include SYSTEM_STRING_H_HEADER
        #else
        #include_next <string.h>
        #endif

and then in config.mak.uname, do something like this:

        ifneq (,$(findstring MINGW,$(uname_S)))
        ifndef SYSTEM_STRING_H_HEADER
        SYSTEM_STRING_H_HEADER = "C:\\llvm\include\string.h"
        endif

        COMPAT_CFLAGS += -DSYSTEM_STRING_H_HEADER=$(SYSTEM_STRING_H_HEADER)
        endif

People who have the system header file at different paths can
further override SYSTEM_STRING_H_HEADER in their config.mak.

That would help compilers targetting mingw that do not support
"#include_next" without spreading the damage to other people's
systems, I think.

I think this is less favorable compared to my last proposed solution.
While my work-around in git-compat-util.h from [1] already is quite
ugly, it's at least in a single place. You solution spreads the code
it multiple place, making it even more ugly and less comprehensible,
IMHO.

[1] http://www.spinics.net/lists/git/msg217546.html

-- 
Sebastian Schuberth