On Fri, Sep 20, 2013 at 08:21:04AM +0200, Piotr Krukowiecki wrote:
I can't think off-hand of a way to do so using preprocessor tricks, and
even if we could, I suspect the result would end up quite ugly.
What I meant was: can we add a test (in t/) which greps git source
code and fails if it
Jeff King p...@peff.net napisał:
On Thu, Sep 19, 2013 at 11:47:51AM +0200, Piotr Krukowiecki wrote:
it still ends up as a single function call). The downside is
that it has
to be remembered at each site that uses strcasecmp, but we do
not use
pointers to standard library functions very
On Wed, Sep 11, 2013 at 9:16 PM, Jeff King p...@peff.net wrote:
I would prefer the static wrapper solution you suggest, though. It
leaves the compiler free to optimize the common case of normal
strcasecmp calls, and only introduces an extra function indirection when
using it as a callback (and
On Thu, Sep 19, 2013 at 9:37 AM, Junio C Hamano ju...@pobox.com wrote:
On Sep 18, 2013 11:08 PM, Piotr Krukowiecki piotr.krukowie...@gmail.com
wrote:
On Wed, Sep 11, 2013 at 9:16 PM, Jeff King p...@peff.net wrote:
I would prefer the static wrapper solution you suggest, though. It
[...]
it
On 12.09.2013 23:31, Jonathan Nieder wrote:
And that's exactly what defining __NO_INLINE__ does. Granted, defining
__NO_INLINE__ in the scope of string.h will also add a #define
strcasecmp _stricmp; but despite it's name, defining __NO_INLINE__
does not imply a performance hit due to functions
On Thu, Sep 19, 2013 at 11:47:51AM +0200, Piotr Krukowiecki wrote:
it still ends up as a single function call). The downside is that it has
to be remembered at each site that uses strcasecmp, but we do not use
pointers to standard library functions very often.
Is it possible to add a
Jeff King p...@peff.net writes:
On Thu, Sep 19, 2013 at 11:47:51AM +0200, Piotr Krukowiecki wrote:
it still ends up as a single function call). The downside is that it has
to be remembered at each site that uses strcasecmp, but we do not use
pointers to standard library functions very
On Thu, Sep 19, 2013 at 03:03:46PM -0700, Junio C Hamano wrote:
But only when someone compiles on MinGW, no?
Yeah. I think a more clear way to phrase the question would be: is there
some trick we can use to booby-trap strcasecmp as a function pointer so
that it fails to compile even on
Jeff King p...@peff.net writes:
... ...and
no, we do not want to go there. Calling it a booby-trap was meant to be
derogatory. :)
OK, I've resurrected the following and queued on 'pu'.
-- 8 --
Subject: [PATCH] mailmap: work around implementations with pure inline
strcasecmp
On some systems
On Thu, Sep 19, 2013 at 03:40:05PM -0700, Junio C Hamano wrote:
Jeff King p...@peff.net writes:
... ...and
no, we do not want to go there. Calling it a booby-trap was meant to be
derogatory. :)
OK, I've resurrected the following and queued on 'pu'.
Looks good to me.
-Peff
--
To
On Tue, Sep 17, 2013 at 11:46 PM, Junio C Hamano gits...@pobox.com 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
On Wed, Sep 18, 2013 at 5:43 AM, Sebastian Schuberth
sschube...@gmail.com wrote:
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.
So I think it's an annoying MinGW
Sebastian Schuberth sschube...@gmail.com writes:
I think this is less favorable compared to my last proposed solution.
That is only needed if you insist to use C preprocessor that does
not understand include_next. That choice is a platform specific
decision (even if you want to use such a
On Tue, Sep 17, 2013 at 6:17 PM, Junio C Hamano gits...@pobox.com wrote:
Keeping the ugliness to deal with the platform issue (i.e. broken
string.h) in one place (e.g. compat/mingw) is far more preferrable
than having a similar ugliness in git-compat-util.h for people on
all other platforms
Sebastian Schuberth sschube...@gmail.com writes:
On Tue, Sep 17, 2013 at 6:17 PM, Junio C Hamano gits...@pobox.com wrote:
Keeping the ugliness to deal with the platform issue (i.e. broken
string.h) in one place (e.g. compat/mingw) is far more preferrable
than having a similar ugliness in
On Sat, Sep 14, 2013 at 12:06 AM, Junio C Hamano gits...@pobox.com 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
On Thu, Sep 12, 2013 at 10:08 PM, Junio C Hamano gits...@pobox.com wrote:
I'm not too happy with the wording either. As I see it, even on MinGW
runtime version 4.0 it's not true that string.h has _only_ inline
definition of strcasecmp; there's also #define strncasecmp
_strnicmp which
On Thu, Sep 12, 2013 at 10:29 PM, Junio C Hamano gits...@pobox.com wrote:
Jeff King p...@peff.net writes:
I think there are basically three classes of solution:
1. Declare __NO_INLINE__ everywhere. I'd worry this might affect other
environments, who would then not inline and lose
Sebastian Schuberth sschube...@gmail.com writes:
On Thu, Sep 12, 2013 at 10:08 PM, Junio C Hamano gits...@pobox.com wrote:
I'm not too happy with the wording either. As I see it, even on MinGW
runtime version 4.0 it's not true that string.h has _only_ inline
definition of strcasecmp; there's
Sebastian Schuberth sschube...@gmail.com writes:
Well, if by everywhere in (1) you mean on all platforms then
you're right. But my patch does not define __NO_INLINE__ globally, but
only at the time string.h / strings.h is included. Afterwards
__NO_INLINE__ is undefined. In that sense,
On Fri, Sep 13, 2013 at 4:26 PM, Junio C Hamano gits...@pobox.com wrote:
Perhaps the real issue is that the header file does not give an
equivalent those who want to take the address of strcasecmp will
get the address of _stricmp instead macro, e.g.
#define strcasecmp _stricmp
or
On Fri, Sep 13, 2013 at 4:37 PM, Junio C Hamano gits...@pobox.com wrote:
Which means people who do want to see that macro defined will be
broken after that section of the header file which unconditionally
undefs it, right?
Right, but luckily you've fixed that in our proposed patch :-)
That
On Fri, Sep 13, 2013 at 12:53 PM, Sebastian Schuberth
sschube...@gmail.com wrote:
+#ifdef __MINGW32__
+#ifdef __NO_INLINE__
Why do you want to push this insane workaround for a clear Mingw bug?
Please have mingw just fix the nasty bug, and the git patch with the
trivial wrapper looks much
Sebastian Schuberth sschube...@gmail.com writes:
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
On Fri, Sep 13, 2013 at 9:56 PM, Linus Torvalds
torva...@linux-foundation.org wrote:
+#ifdef __MINGW32__
+#ifdef __NO_INLINE__
Why do you want to push this insane workaround for a clear Mingw bug?
To be frank, because Git is picking up patches much quicker than MinGW
does, and I want a
On Fri, Sep 13, 2013 at 10:01 PM, Junio C Hamano gits...@pobox.com wrote:
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
Sebastian Schuberth sschube...@gmail.com writes:
On Fri, Sep 13, 2013 at 10:01 PM, Junio C Hamano gits...@pobox.com wrote:
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
Junio C Hamano gits...@pobox.com writes:
You can explicitly include the system header from your compatibility
layer, i.e.
...
and then in config.mak.uname, do something like this:
...
COMPAT_CFLAGS += -DSYSTEM_STRING_H_HEADER=$(SYSTEM_STRING_H_HEADER)
You need to have one level of
On Wed, Sep 11, 2013 at 11:41 PM, Jeff King p...@peff.net wrote:
I'm on Windows using MSYS / MinGW. Since MinGW runtime version 4.0,
string.h contains the following code (see [1]):
#ifndef __NO_INLINE__
__CRT_INLINE int __cdecl __MINGW_NOTHROW
strncasecmp (const char * __sz1, const char *
On Thu, Sep 12, 2013 at 11:36:56AM +0200, Sebastian Schuberth wrote:
Just wondering if that is the root of the problem, or if maybe there is
something else subtle going on. Also, does __CRT_INLINE just turn into
inline, or is there perhaps some other pre-processor magic going on?
This is
John Keeping j...@keeping.me.uk writes:
On Thu, Sep 12, 2013 at 11:36:56AM +0200, Sebastian Schuberth wrote:
Just wondering if that is the root of the problem, or if maybe there is
something else subtle going on. Also, does __CRT_INLINE just turn into
inline, or is there perhaps some other
On Thu, Sep 12, 2013 at 08:37:08AM -0700, Junio C Hamano wrote:
I wonder if GCC has changed it's behaviour to more closely match C99.
Clang as a compatibility article about this sort of issue:
http://clang.llvm.org/compatibility.html#inline
Interesting. The ways the page suggests
Junio C Hamano wrote:
I think we would want something like below.
Looks good to me, but
-- 8 --
Subject: [PATCH] mailmap: work around implementations with pure inline
strcasecmp
On some systems, string.h has _only_ inline definition of strcasecmp
Please specify which system we are
Jeff King p...@peff.net writes:
On Thu, Sep 12, 2013 at 08:37:08AM -0700, Junio C Hamano wrote:
I wonder if GCC has changed it's behaviour to more closely match C99.
Clang as a compatibility article about this sort of issue:
http://clang.llvm.org/compatibility.html#inline
On Thu, Sep 12, 2013 at 11:35:21AM -0700, Junio C Hamano wrote:
- change it to a statis inline;
- remove inline from the definition;
- provide an external (non-inline) def somewhere else;
- compile with gnu899 dialect.
Right, option 3 seems perfectly reasonable to me, as we must
On Thu, Sep 12, 2013 at 8:20 PM, Jeff King p...@peff.net wrote:
I wonder if GCC has changed it's behaviour to more closely match C99.
Clang as a compatibility article about this sort of issue:
http://clang.llvm.org/compatibility.html#inline
Interesting. The ways the page suggests
On Thu, Sep 12, 2013 at 09:46:51PM +0200, Sebastian Schuberth wrote:
Right, option 3 seems perfectly reasonable to me, as we must be prepared
to cope with a decision not to inline the function, and there has to be
_some_ linked implementation. But shouldn't libc be providing an
external,
Sebastian Schuberth sschube...@gmail.com writes:
I'm not too happy with the wording either. As I see it, even on MinGW
runtime version 4.0 it's not true that string.h has _only_ inline
definition of strcasecmp; there's also #define strncasecmp
_strnicmp which effectively provides a non-inline
Jeff King p...@peff.net writes:
I think there are basically three classes of solution:
1. Declare __NO_INLINE__ everywhere. I'd worry this might affect other
environments, who would then not inline and lose performance (but
since it's a non-standard macro, we don't really know
Sebastian Schuberth wrote:
And that's exactly what defining __NO_INLINE__ does. Granted, defining
__NO_INLINE__ in the scope of string.h will also add a #define
strcasecmp _stricmp; but despite it's name, defining __NO_INLINE__
does not imply a performance hit due to functions not being
Sebastian Schuberth wrote:
I'm not too happy with the wording either. As I see it, even on MinGW
runtime version 4.0 it's not true that string.h has _only_ inline
definition of strcasecmp; there's also #define strncasecmp
_strnicmp
I assume you mean #define strcasecmp _stricmp, which is
This is necessary so that read_mailmap() can obtain a pointer to the
function.
Signed-off-by: Sebastian Schuberth sschube...@gmail.com
---
git-compat-util.h | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index
Sebastian Schuberth wrote:
This is necessary so that read_mailmap() can obtain a pointer to the
function.
Hm, what platform has strcasecmp() as an inline function? Is this
allowed by POSIX? Even if it isn't, should we perhaps just work
around it by providing our own thin static function
Sebastian Schuberth sschube...@gmail.com writes:
This is necessary so that read_mailmap() can obtain a pointer to the
function.
Whoa, I didn't think it is even legal for a C library to supply
strcmp() or strcasecmp() that are purely inline you cannot take the
address of. The solution looks a
On Wed, Sep 11, 2013 at 11:29:21AM -0700, Jonathan Nieder wrote:
Sebastian Schuberth wrote:
This is necessary so that read_mailmap() can obtain a pointer to the
function.
Hm, what platform has strcasecmp() as an inline function? Is this
allowed by POSIX? Even if it isn't, should we
On Wed, Sep 11, 2013 at 8:29 PM, Jonathan Nieder jrnie...@gmail.com wrote:
This is necessary so that read_mailmap() can obtain a pointer to the
function.
Hm, what platform has strcasecmp() as an inline function? Is this
allowed by POSIX? Even if it isn't, should we perhaps just work
On Wed, Sep 11, 2013 at 09:59:53PM +0200, Sebastian Schuberth wrote:
On Wed, Sep 11, 2013 at 8:29 PM, Jonathan Nieder jrnie...@gmail.com wrote:
This is necessary so that read_mailmap() can obtain a pointer to the
function.
Hm, what platform has strcasecmp() as an inline function? Is
47 matches
Mail list logo
