Re: [PATCH v2 3/7] win32: override `fspathcmp()` with a directory separator-aware version

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Johannes

On 12/07/2024 00:11, Johannes Schindelin via GitGitGadget wrote:
From: Johannes Schindelin <johannes.schindelin@xxxxxx>

On Windows, the backslash is the directory separator, even if the
forward slash can be used, too, at least since Windows NT.

This means that the paths `a/b` and `a\b` are equivalent, and
`fspathcmp()` needs to be made aware of that fact.

How does fspathncmp() behave? It would be good for the two to match. I've left a couple of thoughts below but I'm not sure they're worth re-rolling for

Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
---
  compat/win32/path-utils.c | 25 +++++++++++++++++++++++++
  compat/win32/path-utils.h |  2 ++
  dir.c                     |  2 +-
  dir.h                     |  2 +-
  git-compat-util.h         |  5 +++++
  5 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/compat/win32/path-utils.c b/compat/win32/path-utils.c
index ebf2f12eb66..af7ef957bbf 100644
--- a/compat/win32/path-utils.c
+++ b/compat/win32/path-utils.c
@@ -50,3 +50,28 @@ int win32_offset_1st_component(const char *path)
return pos + is_dir_sep(*pos) - path;
  }
+
+int win32_fspathcmp(const char *a, const char *b)
+{
+	int diff;
+
+	for (;;) {
+		if (!*a)
+			return !*b ? 0 : -1;

Personally I'd find this easier to read as

	return *b ? -1 : 0;

+		if (!*b)
+			return +1;
+
+		if (is_dir_sep(*a)) {
+			if (!is_dir_sep(*b))
+				return -1;
+			a++;
+			b++;
+			continue;
+		} else if (is_dir_sep(*b))
+			return +1;
+
+		diff = (unsigned char)tolower(*(a++)) - (unsigned char)tolower(*(b++));

There is a subtle difference between this and strcasecmp() because the latter is locale dependent but our tolower() is not because we override the standard library's version. As they're both useless on multibyte locales it probably doesn't make much difference in practice.

Thanks

Phillip

+		if (diff)
+			return diff;
+	}
+}
diff --git a/compat/win32/path-utils.h b/compat/win32/path-utils.h
index 65fa3b9263a..e3c2a79df74 100644
--- a/compat/win32/path-utils.h
+++ b/compat/win32/path-utils.h
@@ -29,5 +29,7 @@ static inline int win32_has_dir_sep(const char *path)
  #define has_dir_sep(path) win32_has_dir_sep(path)
  int win32_offset_1st_component(const char *path);
  #define offset_1st_component win32_offset_1st_component
+int win32_fspathcmp(const char *a, const char *b);
+#define fspathcmp win32_fspathcmp
#endif
diff --git a/dir.c b/dir.c
index b7a6625ebda..37d8e266b2c 100644
--- a/dir.c
+++ b/dir.c
@@ -95,7 +95,7 @@ int count_slashes(const char *s)
  	return cnt;
  }
-int fspathcmp(const char *a, const char *b)
+int git_fspathcmp(const char *a, const char *b)
  {
  	return ignore_case ? strcasecmp(a, b) : strcmp(a, b);
  }
diff --git a/dir.h b/dir.h
index 69a76d8bdd3..947e3d77442 100644
--- a/dir.h
+++ b/dir.h
@@ -541,7 +541,7 @@ int remove_dir_recursively(struct strbuf *path, int flag);
   */
  int remove_path(const char *path);
-int fspathcmp(const char *a, const char *b);
+int git_fspathcmp(const char *a, const char *b);
  int fspatheq(const char *a, const char *b);
  int fspathncmp(const char *a, const char *b, size_t count);
  unsigned int fspathhash(const char *str);
diff --git a/git-compat-util.h b/git-compat-util.h
index ca7678a379d..ac564a68987 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -506,6 +506,11 @@ static inline int git_offset_1st_component(const char *path)
  #define offset_1st_component git_offset_1st_component
  #endif
+
+#ifndef fspathcmp
+#define fspathcmp git_fspathcmp
+#endif
+
  #ifndef is_valid_path
  #define is_valid_path(path) 1
  #endif




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux