{"thread":{"id":"49850","subject":"[PATCH] msvc: Directly use MS version (_stricmp) of strcasecmp","startedAt":"2018-11-18T21:30:01Z","lastAt":"2018-11-20T20:52:26Z","messageCount":6,"participants":["Sven Strickroth","Jeff King","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"363598","messageId":"ad85731f-2c47-f455-c9d3-d47a5fj1e29@cs-ware.de","threadId":"49850","inReplyTo":null,"subject":"[PATCH] msvc: Directly use MS version (_stricmp) of strcasecmp","fromName":"Sven Strickroth","fromEmail":"email@cs-ware.de","sentAt":"2018-11-18T21:02:02Z","receivedAt":"2018-11-18T21:30:01Z","isPatch":true,"sender":{"key":"email@cs-ware.de","avatar":"https://avatars.githubusercontent.com/u/428133?v=4"},"body":"This also removes an implicit conversion from size_t (unsigned) to int (signed).\n\n_stricmp as well as _strnicmp are both available since VS2012.\n\nSigned-off-by: Sven Strickroth \n---\n compat/msvc.h | 8 +-------\n 1 file changed, 1 insertion(+), 7 deletions(-)\n\ndiff --git a/compat/msvc.h b/compat/msvc.h\nindex 580bb55bf4..ea6527f8b6 100644\n--- a/compat/msvc.h\n+++ b/compat/msvc.h\n@@ -14,13 +14,7 @@\n #define ftruncate _chsize\n #define strtoull _strtoui64\n #define strtoll _strtoi64\n-\n-static __inline int strcasecmp (const char *s1, const char *s2)\n-{\n- int size1 = strlen(s1);\n- int sisz2 = strlen(s2);\n- return _strnicmp(s1, s2, sisz2 > size1 ? sisz2 : size1);\n-}\n+#define strcasecmp _stricmp\n\n #undef ERROR\n\n--\n2.19.1.windows.1\n"},{"id":"363599","messageId":"20181118215820.GA14514@sigill.intra.peff.net","threadId":"49850","inReplyTo":"ad85731f-2c47-f455-c9d3-d47a5fj1e29@cs-ware.de","subject":"Re: [PATCH] msvc: Directly use MS version (_stricmp) of strcasecmp","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-18T21:58:20Z","receivedAt":"2018-11-18T21:58:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 18, 2018 at 10:02:02PM +0100, Sven Strickroth wrote:\n\n> This also removes an implicit conversion from size_t (unsigned) to int (signed).\n> \n> _stricmp as well as _strnicmp are both available since VS2012.\n\nOnce upon a time we had problems with taking a function pointer of\nstrcasecmp (to use as a comparator with string_list), so I wondered if\nthat might be part of why it's defined the way it is.\n\nBut the current definition is already inline:\n\n> -\n> -static __inline int strcasecmp (const char *s1, const char *s2)\n> -{\n> - int size1 = strlen(s1);\n> - int sisz2 = strlen(s2);\n> - return _strnicmp(s1, s2, sisz2 > size1 ? sisz2 : size1);\n> -}\n> +#define strcasecmp _stricmp\n\nAnd it seems we worked around this in de2f95ebed (mailmap: work around\nimplementations with pure inline strcasecmp, 2013-09-12). So I don't\nthink there is any blocker there.\n\n(Though of course I have no idea on other portability questions around\n_stricmp(); I'll leave that for Windows folks).\n\n-Peff\n"},{"id":"363607","messageId":"xmqqk1l9esey.fsf@gitster-ct.c.googlers.com","threadId":"49850","inReplyTo":"20181118215820.GA14514@sigill.intra.peff.net","subject":"Re: [PATCH] msvc: Directly use MS version (_stricmp) of strcasecmp","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-19T02:02:13Z","receivedAt":"2018-11-19T02:02:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> And it seems we worked around this in de2f95ebed (mailmap: work around\n> implementations with pure inline strcasecmp, 2013-09-12). So I don't\n> think there is any blocker there.\n>\n> (Though of course I have no idea on other portability questions around\n> _stricmp(); I'll leave that for Windows folks).\n\nLikewise. As to the placement for the replacement #define, the\npatch puts it where the inline version was, but I would think it\nwould work better if it were in the block of #defines, immediately\nnext to #define strncasecmp above.\n.\n\n\n"},{"id":"363638","messageId":"15a7df69-25d8-c168-dd43-a43b7f0d34c1@cs-ware.de","threadId":"49850","inReplyTo":"xmqqk1l9esey.fsf@gitster-ct.c.googlers.com","subject":"[PATCH] msvc: Directly use MS version (_stricmp) of strcasecmp","fromName":"Sven Strickroth","fromEmail":"email@cs-ware.de","sentAt":"2018-11-19T15:14:42Z","receivedAt":"2018-11-19T15:15:43Z","isPatch":true,"sender":{"key":"email@cs-ware.de","avatar":"https://avatars.githubusercontent.com/u/428133?v=4"},"body":"This also removes an implicit conversion from size_t (unsigned) to int (signed).\n\n_stricmp as well as _strnicmp are both available since VS2012.\n\nSigned-off-by: Sven Strickroth \n---\n compat/msvc.h | 8 +-------\n 1 file changed, 1 insertion(+), 7 deletions(-)\n\ndiff --git a/compat/msvc.h b/compat/msvc.h\nindex e6e1a6bbf7..2d558bae14 100644\n--- a/compat/msvc.h\n+++ b/compat/msvc.h\n@@ -14,18 +14,12 @@\n #define inline __inline\n #define __inline__ __inline\n #define __attribute__(x)\n+#define strcasecmp _stricmp\n #define strncasecmp _strnicmp\n #define ftruncate _chsize\n #define strtoull _strtoui64\n #define strtoll _strtoi64\n \n-static __inline int strcasecmp (const char *s1, const char *s2)\n-{\n-\tint size1 = strlen(s1);\n-\tint sisz2 = strlen(s2);\n-\treturn _strnicmp(s1, s2, sisz2 > size1 ? sisz2 : size1);\n-}\n-\n #undef ERROR\n \n #define ftello _ftelli64\n-- \n2.19.1.windows.1\n"},{"id":"363691","messageId":"xmqqefbgcxwl.fsf@gitster-ct.c.googlers.com","threadId":"49850","inReplyTo":"15a7df69-25d8-c168-dd43-a43b7f0d34c1@cs-ware.de","subject":"Re: [PATCH] msvc: Directly use MS version (_stricmp) of strcasecmp","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-20T01:58:50Z","receivedAt":"2018-11-20T01:58:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Strickroth writes:\n\n> This also removes an implicit conversion from size_t (unsigned) to int (signed).\n>\n> _stricmp as well as _strnicmp are both available since VS2012.\n>\n> Signed-off-by: Sven Strickroth \n> ---\n> compat/msvc.h | 8 +-------\n> 1 file changed, 1 insertion(+), 7 deletions(-)\n\nWill apply, thanks.\n\nThe substition from ftello with _ftelli64 does not appear in our\ncodebase yet, but it was easy enough to adjust the patch myself, so\nno need to resend this patch.\n\n> diff --git a/compat/msvc.h b/compat/msvc.h\n> index e6e1a6bbf7..2d558bae14 100644\n> --- a/compat/msvc.h\n> +++ b/compat/msvc.h\n> @@ -14,18 +14,12 @@\n> #define inline __inline\n> #define __inline__ __inline\n> #define __attribute__(x)\n> +#define strcasecmp _stricmp\n> #define strncasecmp _strnicmp\n> #define ftruncate _chsize\n> #define strtoull _strtoui64\n> #define strtoll _strtoi64\n> \n> -static __inline int strcasecmp (const char *s1, const char *s2)\n> -{\n> -\tint size1 = strlen(s1);\n> -\tint sisz2 = strlen(s2);\n> -\treturn _strnicmp(s1, s2, sisz2 > size1 ? sisz2 : size1);\n> -}\n> -\n> #undef ERROR\n> \n> #define ftello _ftelli64\n"},{"id":"363781","messageId":"nycvar.QRO.7.76.6.1811202144390.41@tvgsbejvaqbjf.bet","threadId":"49850","inReplyTo":"xmqqefbgcxwl.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] msvc: Directly use MS version (_stricmp) of strcasecmp","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-20T20:52:00Z","receivedAt":"2018-11-20T20:52:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Tue, 20 Nov 2018, Junio C Hamano wrote:\n\n> Sven Strickroth writes:\n> \n> > This also removes an implicit conversion from size_t (unsigned) to int (signed).\n> >\n> > _stricmp as well as _strnicmp are both available since VS2012.\n\nLooks good to me.\n\n> > Signed-off-by: Sven Strickroth \n> > ---\n> > compat/msvc.h | 8 +-------\n> > 1 file changed, 1 insertion(+), 7 deletions(-)\n> \n> Will apply, thanks.\n> \n> The substition from ftello with _ftelli64 does not appear in our\n> codebase yet, but it was easy enough to adjust the patch myself, so\n> no need to resend this patch.\n\nIndeed, that is only in Git for Windows' code base yet, AFAICT.\n\nFor the record: I am currently holding off from contributing those patches\n(I am talking about the patch series to make Git compile with MS Visual\nC++ on the command line, followed by the patch series to generate project\ndefinitions ready for use with MS Visual Studio) because of the feature\nfreeze. I had hoped to be able to contribute them sooner, but it took Jeff\nHostetler and myself a combined gargantuan effort to reorder and\ndisentangle Git for Windows' branch thicket so that those patches apply\ncleanly on top of git.git's `master`.\n\nHappily, almost all of the prerequisites made it upstream (e.g. the\nnanosecond support for Windows, the patches to require Windows Vista or\nlater, the patch to use CreateHardLink() directly, etc). By my counting,\nonly two, relatively small patch series are left, and both are already\nunder discussion (but on hold, due to the code freeze).\n\nFor interested parties: the current shape of the `visual-studio` patch\nseries can be seen here:\nhttps://github.com/git-for-windows/git/compare/581eb5441089%5E...581eb5441089%5E2\nand the current shape of the `msvc` patch series can be seen here:\nhttps://github.com/git-for-windows/git/compare/e9e7bd2a2485%5E...e9e7bd2a2485%5E2\n\nCiao,\nDscho\n\n> \n> > diff --git a/compat/msvc.h b/compat/msvc.h\n> > index e6e1a6bbf7..2d558bae14 100644\n> > --- a/compat/msvc.h\n> > +++ b/compat/msvc.h\n> > @@ -14,18 +14,12 @@\n> > #define inline __inline\n> > #define __inline__ __inline\n> > #define __attribute__(x)\n> > +#define strcasecmp _stricmp\n> > #define strncasecmp _strnicmp\n> > #define ftruncate _chsize\n> > #define strtoull _strtoui64\n> > #define strtoll _strtoi64\n> > \n> > -static __inline int strcasecmp (const char *s1, const char *s2)\n> > -{\n> > -\tint size1 = strlen(s1);\n> > -\tint sisz2 = strlen(s2);\n> > -\treturn _strnicmp(s1, s2, sisz2 > size1 ? sisz2 : size1);\n> > -}\n> > -\n> > #undef ERROR\n> > \n> > #define ftello _ftelli64\n> \n"}]}