[RFH] renaming strcmp/strncmp-icase
There are these two functions in dir.c that has only a handful of callers outside: int strcmp_icase(const char *a, const char *b); int strncmp_icase(const char *a, const char *b, size_t count); How many of you would think these are about comparing two strings in a case-insensitive way? If you raised your hand (like I did), you were wrong. These do comparison case-insensitively only on a case-insensitive filesystem, and hence calling it makes sense only for pathnames you grabbed out of the filesystem via readdir() (or the user gave us, intending to name paths). To avoid confusion, I think they should be renamed to stress the fact that these are about comparing *PATHS*. As I always say, I am bad at naming things and good suggestions are appreciated. FYI, there are names I thought about and haven't managed to convince myself that they are good. A good name for the non-n variant may be "compare_paths()", but that is used in "combine-diff.c". "compare_pathnames()" may be a compromise. However, either of these makes it hard to come up with a corresponding name for the "n" (counted) variant. The best I could do was "compare_pathnames_counted()", but a nice similarity to strcmp()/strncmp() pair is lost. pathnamecmp()/pathnamencmp() perhaps? I dunno. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFH] renaming strcmp/strncmp-icase
On 2014-09-08 20.52, Junio C Hamano wrote:
> There are these two functions in dir.c that has only a handful of
> callers outside:
>
> int strcmp_icase(const char *a, const char *b);
> int strncmp_icase(const char *a, const char *b, size_t count);
>
> How many of you would think these are about comparing two strings in
> a case-insensitive way?
>
> If you raised your hand (like I did), you were wrong. These do
> comparison case-insensitively only on a case-insensitive filesystem,
> and hence calling it makes sense only for pathnames you grabbed out
> of the filesystem via readdir() (or the user gave us, intending to
> name paths).
>
> To avoid confusion, I think they should be renamed to stress the
> fact that these are about comparing *PATHS*. As I always say, I am
> bad at naming things and good suggestions are appreciated.
>
> FYI, there are names I thought about and haven't managed to convince
> myself that they are good.
>
> A good name for the non-n variant may be "compare_paths()", but that
> is used in "combine-diff.c". "compare_pathnames()" may be a
> compromise.
>
> However, either of these makes it hard to come up with a
> corresponding name for the "n" (counted) variant. The best I could
> do was "compare_pathnames_counted()", but a nice similarity to
> strcmp()/strncmp() pair is lost.
>
> pathnamecmp()/pathnamencmp() perhaps?
>
> I dunno.
And then we have this in name-hash.c:
(Which may explain the "icase" suffix ?)
static int same_name(const struct cache_entry *ce, const char *name, int
namelen, int icase)
{
int len = ce_namelen(ce);
/*
* Always do exact compare, even if we want a case-ignoring comparison;
* we do the quick exact one first, because it will be the common case.
*/
if (len == namelen && !memcmp(name, ce->name, len))
return 1;
if (!icase)
return 0;
return slow_same_name(name, namelen, ce->name, len);
}
---
static int slow_same_name(const char *name1, int len1, const char *name2, int
len2)
{
if (len1 != len2)
return 0;
while (len1) {
unsigned char c1 = *name1++;
unsigned char c2 = *name2++;
len1--;
if (c1 != c2) {
c1 = toupper(c1);
c2 = toupper(c2);
if (c1 != c2)
return 0;
}
}
return 1;
}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFH] renaming strcmp/strncmp-icase
Torsten Bögershausen writes: > And then we have this in name-hash.c: > (Which may explain the "icase" suffix ?) > > static int same_name(const struct cache_entry *ce, const char *name, int > namelen, int icase) As this file-scope static function takes the "icase" as an explicit argument, I do not see anything confusing about it. My complaint was it is confusing that str[n]cmp_icase is not always icase, even though the name of the function implies that it would be to those who haven't looked at its implementation for a while. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFH] renaming strcmp/strncmp-icase
Am 08.09.2014 um 20:52 schrieb Junio C Hamano: There are these two functions in dir.c that has only a handful of callers outside: int strcmp_icase(const char *a, const char *b); int strncmp_icase(const char *a, const char *b, size_t count); How many of you would think these are about comparing two strings in a case-insensitive way? If you raised your hand (like I did), you were wrong. These do comparison case-insensitively only on a case-insensitive filesystem, and hence calling it makes sense only for pathnames you grabbed out of the filesystem via readdir() (or the user gave us, intending to name paths). To avoid confusion, I think they should be renamed to stress the fact that these are about comparing *PATHS*. As I always say, I am bad at naming things and good suggestions are appreciated. pathnamecmp()/pathnamencmp() perhaps? "namen" looks strange to me. How about pathcmp/pathncmp? René -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
