Thu 26 Jan 2006 08:12:01 AM UTC, original submission:
The non-Win32, non-VMS code for directory_contents_hash_cmp bogusly returns equality when two 64 bit inode numbers differ only in the top 32 bits, when the device number is the same and when the machine's natural int width is 32 bits. This isn't the hash function - this is the comparison function. So this can cause two distinct directories to be treated as if they were the same.
You may ask how on earth did I run into this. Well, I think that unlike Cygwin 1.5.18, Cygwin 1.5.19's stat(2) uses the inode numbers returned by Samba, because of this change (and subsequent clarifications):
http://cygwin.com/cgi-bin/cvsweb.cgi/src/winsup/cygwin/fhandler_disk_file.cc.diff?r1=1.147&r2=1.148&cvsroot=src&f=h
Because of the issue documented here:
https://bugzilla.samba.org/show_bug.cgi?id=3287
Some and probably many versions of Samba return the remote inode number in the top 32 bits of the result of eg the Trans2 QUERY_FILE_INFO, Query File Internal Info request, using the bottom 32 bits for the remote device number. Cygwin faithfully copies this into the 64 bit inode number.
make (3.81beta4) is then subtracting one 64 bit inode number from another and assigning the result to what, on my (reasonable) system, is a 32 bit variable:
static int
directory_contents_hash_cmp (const void xv, const void yv)
...
int result;
...
result = x->ino - y->ino;
if (result)
return result;
I know that Windows support is something of a bone of contention but this is, I think, of potentially wider importance, although you'd probably have to be blisteringly unlucky to hit the problem.
64 bit inode numbers aren't such a rarity these days. NFSv3 allows them. They can be especially useful for file systems which support snapshotted versions of files. Whereas 32 bit int still seems to be the norm.
I know it's gruesome but perhaps we could do something like:
/*
* Subtracting ino_t values and assigning the result to
* an int can be lossy.
*/
result = x->ino > y->ino ? 1 : x->ino < y->ino ? -1 : 0;
I see that hash.c doesn't (currently) sort its hash buckets, so we could perhaps get away with:
result = x->ino != y->ino;
If we were going to take that approach, I guess we'd want to change the other subtractions in the same function in the same way. Or leave a comment by the odd man out.
|