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
Jeff King 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 v
On Thu, Sep 19, 2013 at 03:40:05PM -0700, Junio C Hamano wrote:
> Jeff King 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 unsubs
Jeff King 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 (e.g. M
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 compil
Jeff King 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 ve
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
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 9:37 AM, Junio C Hamano wrote:
> On Sep 18, 2013 11:08 PM, "Piotr Krukowiecki"
> wrote:
>>
>> On Wed, Sep 11, 2013 at 9:16 PM, Jeff King wrote:
>> > I would prefer the static wrapper solution you suggest, though. It
[...]
>> > it still ends up as a single function call).
On Wed, Sep 11, 2013 at 9:16 PM, Jeff King 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 even then
On Wed, Sep 18, 2013 at 5:43 AM, Sebastian Schuberth
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 bug, but the reason
On Tue, Sep 17, 2013 at 11:46 PM, Junio C Hamano 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 argu
Sebastian Schuberth writes:
> On Tue, Sep 17, 2013 at 6:17 PM, Junio C Hamano 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
>
On Tue, Sep 17, 2013 at 6:17 PM, Junio C Hamano 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 to see, no?
I
Sebastian Schuberth 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 compiler on a platform i
On Sat, Sep 14, 2013 at 12:06 AM, Junio C Hamano 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_HEA
Junio C Hamano 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 quoting to ke
Sebastian Schuberth writes:
> On Fri, Sep 13, 2013 at 10:01 PM, Junio C Hamano 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 -Icom
On Fri, Sep 13, 2013 at 10:01 PM, Junio C Hamano 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 compiler, whic
On Fri, Sep 13, 2013 at 9:56 PM, Linus Torvalds
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 solution ASAP. Although I of
Sebastian Schuberth 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 bog-standard
technique we a
On Fri, Sep 13, 2013 at 12:53 PM, Sebastian Schuberth
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 simpler than just say
On Fri, Sep 13, 2013 at 4:37 PM, Junio C Hamano 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 is certainly b
On Fri, Sep 13, 2013 at 4:26 PM, Junio C Hamano 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
>>>
>>>
Sebastian Schuberth 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, __NO_INLINE__ is not
Sebastian Schuberth writes:
> On Thu, Sep 12, 2013 at 10:08 PM, Junio C Hamano 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 strncasecm
On Thu, Sep 12, 2013 at 10:29 PM, Junio C Hamano wrote:
> Jeff King 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
>>
On Thu, Sep 12, 2013 at 10:08 PM, Junio C Hamano 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 effectively pr
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", whic
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
Jeff King 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 what it wil
Sebastian Schuberth 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 definition o
On Thu, Sep 12, 2013 at 8:38 PM, Jonathan Nieder wrote:
> 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
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
> > ext
On Thu, Sep 12, 2013 at 8:20 PM, Jeff King 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 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
Jeff King 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
>>
>> Int
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
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
John Keeping 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 pre-pr
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?
>
>
On Wed, Sep 11, 2013 at 11:41 PM, Jeff King 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 * __s
On Wed, Sep 11, 2013 at 09:59:53PM +0200, Sebastian Schuberth wrote:
> On Wed, Sep 11, 2013 at 8:29 PM, Jonathan Nieder 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
> >
On Wed, Sep 11, 2013 at 8:29 PM, Jonathan Nieder 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 pro
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, sho
Sebastian Schuberth 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 bit too large a h
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 wra
This is necessary so that read_mailmap() can obtain a pointer to the
function.
Signed-off-by: Sebastian Schuberth
---
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 be1c494..664305c 100644
--- a/git-co
48 matches
Mail list logo
