{"thread":{"id":"37517","subject":"[RFH] renaming strcmp/strncmp-icase","startedAt":"2014-09-08T18:52:47Z","lastAt":"2014-09-08T21:36:15Z","messageCount":4,"participants":["Junio C Hamano","Torsten Bögershausen","René Scharfe"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"249055","messageId":"xmqqmwaalzb4.fsf@gitster.dls.corp.google.com","threadId":"37517","inReplyTo":null,"subject":"[RFH] renaming strcmp/strncmp-icase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-08T18:52:47Z","receivedAt":"2014-09-08T18:52:47Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"There are these two functions in dir.c that has only a handful of\ncallers outside:\n\n    int strcmp_icase(const char *a, const char *b);\n    int strncmp_icase(const char *a, const char *b, size_t count);\n\nHow many of you would think these are about comparing two strings in\na case-insensitive way?\n\nIf you raised your hand (like I did), you were wrong.  These do\ncomparison case-insensitively only on a case-insensitive filesystem,\nand hence calling it makes sense only for pathnames you grabbed out\nof the filesystem via readdir() (or the user gave us, intending to\nname paths).\n\nTo avoid confusion, I think they should be renamed to stress the\nfact that these are about comparing *PATHS*.  As I always say, I am\nbad at naming things and good suggestions are appreciated.\n\nFYI, there are names I thought about and haven't managed to convince\nmyself that they are good.\n\nA good name for the non-n variant may be \"compare_paths()\", but that\nis used in \"combine-diff.c\".  \"compare_pathnames()\" may be a\ncompromise.\n\nHowever, either of these makes it hard to come up with a\ncorresponding name for the \"n\" (counted) variant.  The best I could\ndo was \"compare_pathnames_counted()\", but a nice similarity to\nstrcmp()/strncmp() pair is lost.\n\npathnamecmp()/pathnamencmp() perhaps?\n\nI dunno.\n"},{"id":"249058","messageId":"540E03F6.3010100@web.de","threadId":"37517","inReplyTo":"xmqqmwaalzb4.fsf@gitster.dls.corp.google.com","subject":"Re: [RFH] renaming strcmp/strncmp-icase","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2014-09-08T19:31:02Z","receivedAt":"2014-09-08T19:31:02Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2014-09-08 20.52, Junio C Hamano wrote:\n> There are these two functions in dir.c that has only a handful of\n> callers outside:\n> \n>     int strcmp_icase(const char *a, const char *b);\n>     int strncmp_icase(const char *a, const char *b, size_t count);\n> \n> How many of you would think these are about comparing two strings in\n> a case-insensitive way?\n> \n> If you raised your hand (like I did), you were wrong.  These do\n> comparison case-insensitively only on a case-insensitive filesystem,\n> and hence calling it makes sense only for pathnames you grabbed out\n> of the filesystem via readdir() (or the user gave us, intending to\n> name paths).\n> \n> To avoid confusion, I think they should be renamed to stress the\n> fact that these are about comparing *PATHS*.  As I always say, I am\n> bad at naming things and good suggestions are appreciated.\n> \n> FYI, there are names I thought about and haven't managed to convince\n> myself that they are good.\n> \n> A good name for the non-n variant may be \"compare_paths()\", but that\n> is used in \"combine-diff.c\".  \"compare_pathnames()\" may be a\n> compromise.\n> \n> However, either of these makes it hard to come up with a\n> corresponding name for the \"n\" (counted) variant.  The best I could\n> do was \"compare_pathnames_counted()\", but a nice similarity to\n> strcmp()/strncmp() pair is lost.\n> \n> pathnamecmp()/pathnamencmp() perhaps?\n> \n> I dunno.\nAnd then we have this in name-hash.c:\n(Which may explain the \"icase\" suffix ?)\n\nstatic int same_name(const struct cache_entry *ce, const char *name, int namelen, int icase)\n{\n\tint len = ce_namelen(ce);\n\n\t/*\n\t * Always do exact compare, even if we want a case-ignoring comparison;\n\t * we do the quick exact one first, because it will be the common case.\n\t */\n\tif (len == namelen && !memcmp(name, ce->name, len))\n\t\treturn 1;\n\n\tif (!icase)\n\t\treturn 0;\n\n\treturn slow_same_name(name, namelen, ce->name, len);\n}\n\n-----------\nstatic int slow_same_name(const char *name1, int len1, const char *name2, int len2)\n{\n\tif (len1 != len2)\n\t\treturn 0;\n\n\twhile (len1) {\n\t\tunsigned char c1 = *name1++;\n\t\tunsigned char c2 = *name2++;\n\t\tlen1--;\n\t\tif (c1 != c2) {\n\t\t\tc1 = toupper(c1);\n\t\t\tc2 = toupper(c2);\n\t\t\tif (c1 != c2)\n\t\t\t\treturn 0;\n\t\t}\n\t}\n\treturn 1;\n}\n"},{"id":"249060","messageId":"xmqqegvlnay6.fsf@gitster.dls.corp.google.com","threadId":"37517","inReplyTo":"540E03F6.3010100@web.de","subject":"Re: [RFH] renaming strcmp/strncmp-icase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-08T19:56:01Z","receivedAt":"2014-09-08T19:56:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n> And then we have this in name-hash.c:\n> (Which may explain the \"icase\" suffix ?)\n>\n> static int same_name(const struct cache_entry *ce, const char *name, int namelen, int icase)\n\nAs this file-scope static function takes the \"icase\" as an explicit\nargument, I do not see anything confusing about it.  My complaint\nwas it is confusing that str[n]cmp_icase is not always icase, even\nthough the name of the function implies that it would be to those\nwho haven't looked at its implementation for a while.\n"},{"id":"249066","messageId":"540E214F.5010708@web.de","threadId":"37517","inReplyTo":"xmqqmwaalzb4.fsf@gitster.dls.corp.google.com","subject":"Re: [RFH] renaming strcmp/strncmp-icase","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2014-09-08T21:36:15Z","receivedAt":"2014-09-08T21:36:15Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 08.09.2014 um 20:52 schrieb Junio C Hamano:\n> There are these two functions in dir.c that has only a handful of\n> callers outside:\n>\n>      int strcmp_icase(const char *a, const char *b);\n>      int strncmp_icase(const char *a, const char *b, size_t count);\n>\n> How many of you would think these are about comparing two strings in\n> a case-insensitive way?\n>\n> If you raised your hand (like I did), you were wrong.  These do\n> comparison case-insensitively only on a case-insensitive filesystem,\n> and hence calling it makes sense only for pathnames you grabbed out\n> of the filesystem via readdir() (or the user gave us, intending to\n> name paths).\n>\n> To avoid confusion, I think they should be renamed to stress the\n> fact that these are about comparing *PATHS*.  As I always say, I am\n> bad at naming things and good suggestions are appreciated.\n\n> pathnamecmp()/pathnamencmp() perhaps?\n\n\"namen\" looks strange to me.  How about pathcmp/pathncmp?\n\nRené\n"}]}