diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index b37e5f670..ce0a3f7a9 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,22 @@ +2010-01-29 Corinna Vinschen + + Throughout, use FileBothDirectoryInformation info class rather than + FileDirectoryInformation info class to avoid problems with incomplete + filesystem implementations. Fix comments accordingly. + * fhandler_disk_file.cc (fhandler_disk_file::readdir_helper): Set + fname->Length to 0 in error case to avoid potential crash in debug + output. + (fhandler_disk_file::readdir): Try to speed up the working default case. + Check for STATUS_INVALID_NETWORK_RESPONSE as potential status value + returned by filesystems not implementing FileIdBothDirectoryInformation. + * ntdll.h (STATUS_INVALID_NETWORK_RESPONSE): Define. + (FILE_BOTH_DIRECTORY_INFORMATION): Rename to official name. + * path.cc (symlink_info::check): Don't request FILE_READ_EA access, it's + not required for NFS. Try to speed up the working default case. Check + for STATUS_INVALID_NETWORK_RESPONSE as potential status value returned + by filesystems not supporting non-NULL EA parameters. Fix the way + fs.update is called. Improve debug output. + 2010-01-28 Corinna Vinschen * Makefile.in (tags, ctags, CTAGS)): Add rules to create tags file. diff --git a/winsup/cygwin/fhandler_disk_file.cc b/winsup/cygwin/fhandler_disk_file.cc index 8be74add5..b98c70357 100644 --- a/winsup/cygwin/fhandler_disk_file.cc +++ b/winsup/cygwin/fhandler_disk_file.cc @@ -211,11 +211,11 @@ path_conv::ndisk_links (DWORD nNumberOfLinks) unsigned count = 0; bool first = true; - PFILE_DIRECTORY_INFORMATION fdibuf = (PFILE_DIRECTORY_INFORMATION) + PFILE_BOTH_DIRECTORY_INFORMATION fdibuf = (PFILE_BOTH_DIRECTORY_INFORMATION) alloca (65536); __DIR_mounts *dir = new __DIR_mounts (normalized_path); while (NT_SUCCESS (NtQueryDirectoryFile (fh, NULL, NULL, NULL, &io, fdibuf, - 65536, FileDirectoryInformation, + 65536, FileBothDirectoryInformation, FALSE, NULL, first))) { if (first) @@ -227,9 +227,9 @@ path_conv::ndisk_links (DWORD nNumberOfLinks) if (fdibuf->FileNameLength != 2 || fdibuf->FileName[0] != L'.') count = 2; } - for (PFILE_DIRECTORY_INFORMATION pfdi = fdibuf; + for (PFILE_BOTH_DIRECTORY_INFORMATION pfdi = fdibuf; pfdi; - pfdi = (PFILE_DIRECTORY_INFORMATION) + pfdi = (PFILE_BOTH_DIRECTORY_INFORMATION) (pfdi->NextEntryOffset ? (PBYTE) pfdi + pfdi->NextEntryOffset : NULL)) { @@ -434,7 +434,7 @@ fhandler_base::fstat_by_name (struct __stat64 *buf) else if (NT_SUCCESS (status = NtQueryDirectoryFile (dir, NULL, NULL, NULL, &io, &fdi_buf.fdi, sizeof fdi_buf, - FileDirectoryInformation, + FileBothDirectoryInformation, TRUE, &basename, TRUE))) FileId.QuadPart = 0; /* get_ino is called in fstat_helper. */ if (!NT_SUCCESS (status)) @@ -1665,7 +1665,7 @@ fhandler_disk_file::opendir (int fd) OS/FS combinations (say, Win2K/CDFS or so). Instead of testing in readdir for yet another error code, let's use FileIdBothDirectoryInformation only on filesystems supporting - persistent ACLs, FileDirectoryInformation otherwise. + persistent ACLs, FileBothDirectoryInformation otherwise. NFS clients hide dangling symlinks from directory queries, unless you use the FileNamesInformation info class. @@ -1770,7 +1770,10 @@ fhandler_disk_file::readdir_helper (DIR *dir, dirent *de, DWORD w32_err, if ((de->d_ino = d_mounts (dir)->check_missing_mount (fname))) added = true; if (!added) - return geterrno_from_win_error (w32_err); + { + fname->Length = 0; + return geterrno_from_win_error (w32_err); + } attr = 0; dir->__flags &= ~dirent_set_d_ino; @@ -1891,30 +1894,32 @@ fhandler_disk_file::readdir (DIR *dir, dirent *de) which return STATUS_NOT_SUPPORTED rather than handling this info class. We just fall back to using a standard directory query in this case and note this case using the dirent_get_d_ino flag. */ - if (status == STATUS_INVALID_LEVEL - || status == STATUS_NOT_SUPPORTED - || status == STATUS_INVALID_PARAMETER - || status == STATUS_INVALID_INFO_CLASS) + if (!NT_SUCCESS (status) && status != STATUS_NO_MORE_FILES + && (status == STATUS_INVALID_LEVEL + || status == STATUS_NOT_SUPPORTED + || status == STATUS_INVALID_PARAMETER + || status == STATUS_INVALID_NETWORK_RESPONSE + || status == STATUS_INVALID_INFO_CLASS)) dir->__flags &= ~dirent_get_d_ino; /* Something weird happens on Samba up to version 3.0.21c, which is fixed in 3.0.22. FileIdBothDirectoryInformation seems to work nicely, but only up to the 128th entry in the directory. After reaching this entry, the next call to NtQueryDirectoryFile (FileIdBothDirectoryInformation) returns STATUS_INVALID_LEVEL. - Why should we care, we can just switch to FileDirectoryInformation, - isn't it? Nope! The next call to - NtQueryDirectoryFile(FileDirectoryInformation) - actually returns STATUS_NO_MORE_FILES, regardless how many files - are left unread in the directory. This does not happen when using - FileDirectoryInformation right from the start, but since + Why should we care, we can just switch to + FileBothDirectoryInformation, isn't it? Nope! The next call to + NtQueryDirectoryFile(FileBothDirectoryInformation) actually + returns STATUS_NO_MORE_FILES, regardless how many files are left + unread in the directory. This does not happen when using + FileBothDirectoryInformation right from the start, but since we can't decide whether the server we're talking with has this bug or not, we end up serving Samba shares always in the slow - mode using FileDirectoryInformation. So, what we do here is + mode using FileBothDirectoryInformation. So, what we do here is to implement the solution suggested by Andrew Tridgell, we just reread all entries up to dir->d_position using - FileDirectoryInformation. + FileBothDirectoryInformation. However, We do *not* mark this server as broken and fall back to - using FileDirectoryInformation further on. This would slow + using FileBothDirectoryInformation further on. This would slow down every access to such a server, even for directories under 128 entries. Also, bigger dirs only suffer from one additional call per full directory scan, which shouldn't be too big a hit. @@ -1929,7 +1934,7 @@ fhandler_disk_file::readdir (DIR *dir, dirent *de) status = NtQueryDirectoryFile (get_handle (), NULL, NULL, NULL, &io, d_cache (dir), DIR_BUF_SIZE, - FileDirectoryInformation, + FileBothDirectoryInformation, FALSE, NULL, cnt == 0); if (!NT_SUCCESS (status)) goto go_ahead; @@ -1949,7 +1954,7 @@ fhandler_disk_file::readdir (DIR *dir, dirent *de) d_cache (dir), DIR_BUF_SIZE, (dir->__flags & dirent_nfs_d_ino) ? FileNamesInformation - : FileDirectoryInformation, + : FileBothDirectoryInformation, FALSE, NULL, dir->__d_position == 0); } @@ -1982,9 +1987,11 @@ go_ahead: } else { - FileName = ((PFILE_DIRECTORY_INFORMATION) buf)->FileName; - FileNameLength = ((PFILE_DIRECTORY_INFORMATION) buf)->FileNameLength; - FileAttributes = ((PFILE_DIRECTORY_INFORMATION) buf)->FileAttributes; + FileName = ((PFILE_BOTH_DIRECTORY_INFORMATION) buf)->FileName; + FileNameLength = + ((PFILE_BOTH_DIRECTORY_INFORMATION) buf)->FileNameLength; + FileAttributes = + ((PFILE_BOTH_DIRECTORY_INFORMATION) buf)->FileAttributes; } RtlInitCountedUnicodeString (&fname, FileName, FileNameLength); de->d_ino = d_mounts (dir)->check_mount (&fname, de->d_ino); diff --git a/winsup/cygwin/ntdll.h b/winsup/cygwin/ntdll.h index 0eeeb544e..465d52782 100644 --- a/winsup/cygwin/ntdll.h +++ b/winsup/cygwin/ntdll.h @@ -1,7 +1,7 @@ /* ntdll.h. Contains ntdll specific stuff not defined elsewhere. Copyright 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, - 2009 Red Hat, Inc. + 2009, 2010 Red Hat, Inc. This file is part of Cygwin. @@ -39,6 +39,7 @@ #define STATUS_DISK_FULL ((NTSTATUS) 0xc000007f) #define STATUS_WORKING_SET_QUOTA ((NTSTATUS) 0xc00000a1) #define STATUS_NOT_SUPPORTED ((NTSTATUS) 0xc00000bb) +#define STATUS_INVALID_NETWORK_RESPONSE ((NTSTATUS) 0xc00000c3) #define STATUS_DIRECTORY_NOT_EMPTY ((NTSTATUS) 0xc0000101) #define STATUS_CANNOT_DELETE ((NTSTATUS) 0xc0000121) #define STATUS_INVALID_LEVEL ((NTSTATUS) 0xc0000148) @@ -153,7 +154,7 @@ typedef struct _FILE_DIRECTORY_INFORMATION { WCHAR FileName[1]; } FILE_DIRECTORY_INFORMATION, *PFILE_DIRECTORY_INFORMATION; -typedef struct _FILE_BOTH_DIR_INFORMATION +typedef struct _FILE_BOTH_DIRECTORY_INFORMATION { ULONG NextEntryOffset; ULONG FileIndex; @@ -169,7 +170,7 @@ typedef struct _FILE_BOTH_DIR_INFORMATION CCHAR ShortNameLength; WCHAR ShortName[12]; WCHAR FileName[1]; -} FILE_BOTH_DIR_INFORMATION, *PFILE_BOTH_DIR_INFORMATION; +} FILE_BOTH_DIRECTORY_INFORMATION, *PFILE_BOTH_DIRECTORY_INFORMATION; typedef struct _FILE_ID_BOTH_DIR_INFORMATION { diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc index 8ad8d2e52..77a7034f8 100644 --- a/winsup/cygwin/path.cc +++ b/winsup/cygwin/path.cc @@ -514,7 +514,7 @@ getfileattr (const char *path, bool caseinsensitive) /* path has to be always ab directory query. */ UNICODE_STRING dirname, basename; HANDLE dir; - FILE_DIRECTORY_INFORMATION fdi; + FILE_BOTH_DIRECTORY_INFORMATION fdi; RtlSplitUnicodePath (&upath, &dirname, &basename); InitializeObjectAttributes (&attr, &dirname, @@ -529,7 +529,7 @@ getfileattr (const char *path, bool caseinsensitive) /* path has to be always ab { status = NtQueryDirectoryFile (dir, NULL, NULL, 0, &io, &fdi, sizeof fdi, - FileDirectoryInformation, + FileBothDirectoryInformation, TRUE, &basename, TRUE); NtClose (dir); if (NT_SUCCESS (status) || status == STATUS_BUFFER_OVERFLOW) @@ -2209,18 +2209,22 @@ symlink_info::check (char *path, const suffix_info *suffixes, unsigned opt, Fortunately it's ignored on most other file systems so we don't have to special case NFS too much. */ status = NtCreateFile (&h, - READ_CONTROL | FILE_READ_ATTRIBUTES | FILE_READ_EA, + READ_CONTROL | FILE_READ_ATTRIBUTES, &attr, &io, NULL, 0, FILE_SHARE_VALID_FLAGS, FILE_OPEN, FILE_OPEN_REPARSE_POINT | FILE_OPEN_FOR_BACKUP_INTENT, eabuf, easize); + debug_printf ("%p = NtCreateFile (%S)", status, &upath); /* No right to access EAs or EAs not supported? */ - if (status == STATUS_ACCESS_DENIED || status == STATUS_EAS_NOT_SUPPORTED - || status == STATUS_NOT_SUPPORTED - /* Or a bug in Samba 3.2.x (x <= 7) when accessing a share's root dir - which has EAs enabled? */ - || status == STATUS_INVALID_PARAMETER) + if (!NT_SUCCESS (status) + && (status == STATUS_ACCESS_DENIED + || status == STATUS_EAS_NOT_SUPPORTED + || status == STATUS_NOT_SUPPORTED + || status == STATUS_INVALID_NETWORK_RESPONSE + /* Or a bug in Samba 3.2.x (x <= 7) when accessing a share's + root dir which has EAs enabled? */ + || status == STATUS_INVALID_PARAMETER)) { no_ea = true; /* If EAs are not supported, there's no sense to check them again @@ -2235,6 +2239,7 @@ symlink_info::check (char *path, const suffix_info *suffixes, unsigned opt, &attr, &io, FILE_SHARE_VALID_FLAGS, FILE_OPEN_REPARSE_POINT | FILE_OPEN_FOR_BACKUP_INTENT); + debug_printf ("%p = NtOpenFile (no-EA, %S)", status, &upath); } if (status == STATUS_OBJECT_NAME_NOT_FOUND && ci_flag == 0 && wincap.has_broken_udf ()) @@ -2247,6 +2252,7 @@ symlink_info::check (char *path, const suffix_info *suffixes, unsigned opt, &attr, &io, FILE_SHARE_VALID_FLAGS, FILE_OPEN_REPARSE_POINT | FILE_OPEN_FOR_BACKUP_INTENT); + debug_printf ("%p = NtOpenFile (broken-UDF, %S)", status, &upath); attr.Attributes = 0; if (NT_SUCCESS (status)) { @@ -2261,12 +2267,10 @@ symlink_info::check (char *path, const suffix_info *suffixes, unsigned opt, } } - /* Check file system while we're having the file open anyway. - This speeds up path_conv noticably (~10%). */ - if (!fs_update_called) - fs.update (&upath, h); - if (NT_SUCCESS (status) + /* Check file system while we're having the file open anyway. + This speeds up path_conv noticably (~10%). */ + && (fs_update_called || fs.update (&upath, h)) && NT_SUCCESS (status = fs.has_buggy_basic_info () ? NtQueryAttributesFile (&attr, &fbi) : NtQueryInformationFile (h, &io, &fbi, sizeof fbi, @@ -2306,7 +2310,7 @@ symlink_info::check (char *path, const suffix_info *suffixes, unsigned opt, OBJECT_ATTRIBUTES dattr; HANDLE dir; struct { - FILE_DIRECTORY_INFORMATION fdi; + FILE_BOTH_DIRECTORY_INFORMATION fdi; WCHAR dummy_buf[NAME_MAX + 1]; } fdi_buf; @@ -2332,7 +2336,7 @@ symlink_info::check (char *path, const suffix_info *suffixes, unsigned opt, { status = NtQueryDirectoryFile (dir, NULL, NULL, NULL, &io, &fdi_buf, sizeof fdi_buf, - FileDirectoryInformation, + FileBothDirectoryInformation, TRUE, &basename, TRUE); /* Take the opportunity to check file system while we're having the handle to the parent dir. */