Re: [PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems

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

 



On 05/08/2014 08:37 AM, Johannes Sixt wrote:
Am 5/7/2014 19:46, schrieb Junio C Hamano:
David Turner <dturner@xxxxxxxxxxxxxxxx> writes:

On Wed, 2014-05-07 at 08:17 +0200, Johannes Sixt wrote:
  		} else if (cache_name_pos(src, length) < 0)
  			bad = _("not under version control");
-		else if (lstat(dst, &st) == 0) {
+		else if (lstat(dst, &dst_st) == 0 &&
+			 (src_st.st_ino != dst_st.st_ino ||
+			  (src_st.st_ino == 0 && strcasecmp(src, dst)))) {
Don't do that. st_ino is zero on Windows only because we do not spend time
to fill in the field. Don't use it as an indicator for a case-insensitive
file system; zero may be a valid inode number on other systems.
I don't think it is a problem if zero is a valid inode.  The only thing
that happens when there is a zero inode, is that we have to compare
filenames.  The inode check is just an optimization to avoid doing a
bunch of strcasecmp on systems that don't have to.
Am I correct to rephrase you that the code assumes that any
filesystem that cannot give unique inum to different files will use
0 as the placeholder inum, so if src/dst share the same non-zero
inum, it is guaranteed that is not a placeholder and we know they
are different files without comparing the two paths?
"If src and dst share the same non-zero inum, it is guaranteed that it is
not a placeholder and we know they are the same file" is more correct.

What if both inums are zero? Can this happen on any sane POSIX system? I
don't know, but my gut feeling is that inode zero is too special to be
allocated for files or directories.

In that case, it is safe to assume that the st_ino field is just a
placeholder when it is zero, and we have to compare the file name. Then we
can either assume that this happens only for our emulation layer on MinGW
(and the comparison can be case-insensitive) or choose the comparison mode
based on core.ignorecase. This patch does the former, but I think we
should do the latter.
Whatever we do, we should "protect" the strcasecmp() with ignore_case:

!ignore_case || strcasecmp(src, dst)

(And once that is done, you don't need to look at st_ino at all)


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




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