[RFH] renaming strcmp/strncmp-icase

[RFH] renaming strcmp/strncmp-icase

[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.

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.

Re: [RFH] renaming strcmp/strncmp-icase

[Torsten Bögershausen]

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;
}

Re: [RFH] renaming strcmp/strncmp-icase

[Junio C Hamano]

Torsten Bögershausen <tboegi@web.de> 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.

Re: [RFH] renaming strcmp/strncmp-icase

[René Scharfe]

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é