threads / patch / 49850

patchmsvc: Directly use MS version (_stricmp) of strcasecmp

Subject: [PATCH] msvc: Directly use MS version (_stricmp) of strcasecmp

## tl;dr

6 messages between Nov 18, 2018 and Nov 20, 2018. Diffs are folded; open one to read it.

replies: 5people: 4as markdown or json

Sven Strickroth· Nov 18, 2018, 21:02 UTC · lore
This also removes an implicit conversion from size_t (unsigned) to int (signed).
_stricmp as well as _strnicmp are both available since VS2012.
Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
 compat/msvc.h | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
Show changes to compat/msvc.h +1 −8
diff --git a/compat/msvc.h b/compat/msvc.h
index 580bb55bf4..ea6527f8b6 100644
--- a/compat/msvc.h
+++ b/compat/msvc.h
@@ -14,13 +14,7 @@
 #define ftruncate    _chsize
 #define strtoull     _strtoui64
 #define strtoll      _strtoi64
-
-static __inline int strcasecmp (const char *s1, const char *s2)
-{
-       int size1 = strlen(s1);
-       int sisz2 = strlen(s2);
-       return _strnicmp(s1, s2, sisz2 > size1 ? sisz2 : size1);
-}
+#define strcasecmp   _stricmp

 #undef ERROR

--
2.19.1.windows.1
Jeff King· Nov 18, 2018, 21:58 UTC · re: Sven Strickroth · lore

Re: [PATCH] msvc: Directly use MS version (_stricmp) of strcasecmp

On Sun, Nov 18, 2018 at 10:02:02PM +0100, Sven Strickroth wrote:
> This also removes an implicit conversion from size_t (unsigned) to int (signed).
> 
> _stricmp as well as _strnicmp are both available since VS2012.

Once upon a time we had problems with taking a function pointer of strcasecmp (to use as a comparator with string_list), so I wondered if that might be part of why it's defined the way it is.

But the current definition is already inline:
Show 8 quoted lines
> -
> -static __inline int strcasecmp (const char *s1, const char *s2)
> -{
> -       int size1 = strlen(s1);
> -       int sisz2 = strlen(s2);
> -       return _strnicmp(s1, s2, sisz2 > size1 ? sisz2 : size1);
> -}
> +#define strcasecmp   _stricmp

And it seems we worked around this in de2f95ebed (mailmap: work around implementations with pure inline strcasecmp, 2013-09-12). So I don't think there is any blocker there.

(Though of course I have no idea on other portability questions around _stricmp(); I'll leave that for Windows folks).

-Peff
Junio C Hamano· Nov 19, 2018, 02:02 UTC · re: Jeff King · lore

Re: [PATCH] msvc: Directly use MS version (_stricmp) of strcasecmp

Jeff King <peff@peff.net> writes:
Show 6 quoted lines
> And it seems we worked around this in de2f95ebed (mailmap: work around
> implementations with pure inline strcasecmp, 2013-09-12). So I don't
> think there is any blocker there.
>
> (Though of course I have no idea on other portability questions around
> _stricmp(); I'll leave that for Windows folks).

Likewise. As to the placement for the replacement #define, the patch puts it where the inline version was, but I would think it would work better if it were in the block of #defines, immediately next to #define strncasecmp above. .

Sven Strickroth· Nov 19, 2018, 15:14 UTC · re: Junio C Hamano · lore
This also removes an implicit conversion from size_t (unsigned) to int (signed).
_stricmp as well as _strnicmp are both available since VS2012.
Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
 compat/msvc.h | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
Show changes to compat/msvc.h +1 −7
diff --git a/compat/msvc.h b/compat/msvc.h
index e6e1a6bbf7..2d558bae14 100644
--- a/compat/msvc.h
+++ b/compat/msvc.h
@@ -14,18 +14,12 @@
 #define inline __inline
 #define __inline__ __inline
 #define __attribute__(x)
+#define strcasecmp   _stricmp
 #define strncasecmp  _strnicmp
 #define ftruncate    _chsize
 #define strtoull     _strtoui64
 #define strtoll      _strtoi64
 
-static __inline int strcasecmp (const char *s1, const char *s2)
-{
-	int size1 = strlen(s1);
-	int sisz2 = strlen(s2);
-	return _strnicmp(s1, s2, sisz2 > size1 ? sisz2 : size1);
-}
-
 #undef ERROR
 
 #define ftello _ftelli64
-- 
2.19.1.windows.1
Junio C Hamano· Nov 20, 2018, 01:58 UTC · re: Sven Strickroth · lore

Re: [PATCH] msvc: Directly use MS version (_stricmp) of strcasecmp

Sven Strickroth <email@cs-ware.de> writes:
Show 8 quoted lines
> This also removes an implicit conversion from size_t (unsigned) to int (signed).
>
> _stricmp as well as _strnicmp are both available since VS2012.
>
> Signed-off-by: Sven Strickroth <email@cs-ware.de>
> ---
>  compat/msvc.h | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
Will apply, thanks.

The substition from ftello with _ftelli64 does not appear in our codebase yet, but it was easy enough to adjust the patch myself, so no need to resend this patch.

Show 24 quoted lines
> diff --git a/compat/msvc.h b/compat/msvc.h
> index e6e1a6bbf7..2d558bae14 100644
> --- a/compat/msvc.h
> +++ b/compat/msvc.h
> @@ -14,18 +14,12 @@
>  #define inline __inline
>  #define __inline__ __inline
>  #define __attribute__(x)
> +#define strcasecmp   _stricmp
>  #define strncasecmp  _strnicmp
>  #define ftruncate    _chsize
>  #define strtoull     _strtoui64
>  #define strtoll      _strtoi64
>  
> -static __inline int strcasecmp (const char *s1, const char *s2)
> -{
> -	int size1 = strlen(s1);
> -	int sisz2 = strlen(s2);
> -	return _strnicmp(s1, s2, sisz2 > size1 ? sisz2 : size1);
> -}
> -
>  #undef ERROR
>  
>  #define ftello _ftelli64
Johannes Schindelin· Nov 20, 2018, 20:52 UTC · re: Junio C Hamano · lore

Re: [PATCH] msvc: Directly use MS version (_stricmp) of strcasecmp

Hi Junio,
On Tue, 20 Nov 2018, Junio C Hamano wrote:
Show 5 quoted lines
> Sven Strickroth <email@cs-ware.de> writes:
> 
> > This also removes an implicit conversion from size_t (unsigned) to int (signed).
> >
> > _stricmp as well as _strnicmp are both available since VS2012.
Looks good to me.
Show 10 quoted lines
> > Signed-off-by: Sven Strickroth <email@cs-ware.de>
> > ---
> >  compat/msvc.h | 8 +-------
> >  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> Will apply, thanks.
> 
> The substition from ftello with _ftelli64 does not appear in our
> codebase yet, but it was easy enough to adjust the patch myself, so
> no need to resend this patch.
Indeed, that is only in Git for Windows' code base yet, AFAICT.

For the record: I am currently holding off from contributing those patches (I am talking about the patch series to make Git compile with MS Visual C++ on the command line, followed by the patch series to generate project definitions ready for use with MS Visual Studio) because of the feature freeze. I had hoped to be able to contribute them sooner, but it took Jeff Hostetler and myself a combined gargantuan effort to reorder and disentangle Git for Windows' branch thicket so that those patches apply cleanly on top of git.git's `master`.

Happily, almost all of the prerequisites made it upstream (e.g. the nanosecond support for Windows, the patches to require Windows Vista or later, the patch to use CreateHardLink() directly, etc). By my counting, only two, relatively small patch series are left, and both are already under discussion (but on hold, due to the code freeze).

For interested parties: the current shape of the `visual-studio` patch series can be seen here: https://github.com/git-for-windows/git/compare/581eb5441089%5E...581eb5441089%5E2 and the current shape of the `msvc` patch series can be seen here: https://github.com/git-for-windows/git/compare/e9e7bd2a2485%5E...e9e7bd2a2485%5E2

Ciao, Dscho

Show 26 quoted lines
> 
> > diff --git a/compat/msvc.h b/compat/msvc.h
> > index e6e1a6bbf7..2d558bae14 100644
> > --- a/compat/msvc.h
> > +++ b/compat/msvc.h
> > @@ -14,18 +14,12 @@
> >  #define inline __inline
> >  #define __inline__ __inline
> >  #define __attribute__(x)
> > +#define strcasecmp   _stricmp
> >  #define strncasecmp  _strnicmp
> >  #define ftruncate    _chsize
> >  #define strtoull     _strtoui64
> >  #define strtoll      _strtoi64
> >  
> > -static __inline int strcasecmp (const char *s1, const char *s2)
> > -{
> > -	int size1 = strlen(s1);
> > -	int sisz2 = strlen(s2);
> > -	return _strnicmp(s1, s2, sisz2 > size1 ? sisz2 : size1);
> > -}
> > -
> >  #undef ERROR
> >  
> >  #define ftello _ftelli64
> 

← back to recent threads