OSDN Git Service

* fhandler.cc (fhandler_base::open): Drop local create_options variable.
authorcorinna <corinna>
Fri, 17 Jun 2011 11:04:41 +0000 (11:04 +0000)
committercorinna <corinna>
Fri, 17 Jun 2011 11:04:41 +0000 (11:04 +0000)
Use options member instead.
* fhandler.h (class fhandler_base): Change type of access member to
ACCESS_MASK.  Change get_access and set_access methods accordingly.
Add options member.  Add get_options and set_options methods.
(class fhandler_disk_file): Add prw_handle.
(fhandler_disk_file::prw_open): Declare.
(fhandler_disk_file::close): Declare.
(fhandler_disk_file::dup): Declare.
(fhandler_disk_file::fixup_after_fork): Declare.
* fhandler_disk_file.cc (fhandler_disk_file::fhandler_disk_file):
Initialize prw_handle to NULL.
(fhandler_disk_file::close): Close prw_handle.
(fhandler_disk_file::dup): New method.
(fhandler_disk_file::fixup_after_fork): Set prw_handle to NULL since
prw_handle is not inherited.
(fhandler_disk_file::prw_open): New method.  Add long comment to
explain current behaviour.
(fhandler_disk_file::pread): Revert previous change.  Change to use
prw_handle if possible.
(fhandler_disk_file::pwrite): Change to use prw_handle if possible.

winsup/cygwin/ChangeLog
winsup/cygwin/fhandler.cc
winsup/cygwin/fhandler.h
winsup/cygwin/fhandler_disk_file.cc

index 3b9b57b..9ee89e9 100644 (file)
@@ -1,5 +1,29 @@
 2011-06-17  Corinna Vinschen  <corinna@vinschen.de>
 
+       * fhandler.cc (fhandler_base::open): Drop local create_options variable.
+       Use options member instead.
+       * fhandler.h (class fhandler_base): Change type of access member to
+       ACCESS_MASK.  Change get_access and set_access methods accordingly.
+       Add options member.  Add get_options and set_options methods.
+       (class fhandler_disk_file): Add prw_handle.
+       (fhandler_disk_file::prw_open): Declare.
+       (fhandler_disk_file::close): Declare.
+       (fhandler_disk_file::dup): Declare.
+       (fhandler_disk_file::fixup_after_fork): Declare.
+       * fhandler_disk_file.cc (fhandler_disk_file::fhandler_disk_file):
+       Initialize prw_handle to NULL.
+       (fhandler_disk_file::close): Close prw_handle.
+       (fhandler_disk_file::dup): New method.
+       (fhandler_disk_file::fixup_after_fork): Set prw_handle to NULL since
+       prw_handle is not inherited.
+       (fhandler_disk_file::prw_open): New method.  Add long comment to
+       explain current behaviour.
+       (fhandler_disk_file::pread): Revert previous change.  Change to use
+       prw_handle if possible.
+       (fhandler_disk_file::pwrite): Change to use prw_handle if possible.
+
+2011-06-17  Corinna Vinschen  <corinna@vinschen.de>
+
        * dcrt0.cc (dll_crt0_1): Call strace.dll_info after call to pinfo_init.
        * strace.cc (strace::hello): Drop printing DLL information here since
        application info is not always available at this point.
index 4ea8497..7e01f67 100644 (file)
@@ -492,7 +492,6 @@ fhandler_base::open (int flags, mode_t mode)
   ULONG file_attributes = 0;
   ULONG shared = (get_major () == DEV_TAPE_MAJOR ? 0 : FILE_SHARE_VALID_FLAGS);
   ULONG create_disposition;
-  ULONG create_options = FILE_OPEN_FOR_BACKUP_INTENT;
   OBJECT_ATTRIBUTES attr;
   IO_STATUS_BLOCK io;
   NTSTATUS status;
@@ -503,6 +502,7 @@ fhandler_base::open (int flags, mode_t mode)
 
   pc.get_object_attr (attr, *sec_none_cloexec (flags));
 
+  options = FILE_OPEN_FOR_BACKUP_INTENT;
   switch (query_open ())
     {
       case query_read_control:
@@ -528,12 +528,12 @@ fhandler_base::open (int flags, mode_t mode)
        else
          access = GENERIC_READ | GENERIC_WRITE;
        if (flags & O_SYNC)
-         create_options |= FILE_WRITE_THROUGH;
+         options |= FILE_WRITE_THROUGH;
        if (flags & O_DIRECT)
-         create_options |= FILE_NO_INTERMEDIATE_BUFFERING;
+         options |= FILE_NO_INTERMEDIATE_BUFFERING;
        if (get_major () != DEV_SERIAL_MAJOR && get_major () != DEV_TAPE_MAJOR)
          {
-           create_options |= FILE_SYNCHRONOUS_IO_NONALERT;
+           options |= FILE_SYNCHRONOUS_IO_NONALERT;
            access |= SYNCHRONIZE;
          }
        break;
@@ -574,7 +574,7 @@ fhandler_base::open (int flags, mode_t mode)
       /* Add the reparse point flag to native symlinks, otherwise we open the
         target, not the symlink.  This would break lstat. */
       if (pc.is_rep_symlink ())
-       create_options |= FILE_OPEN_REPARSE_POINT;
+       options |= FILE_OPEN_REPARSE_POINT;
 
       /* Starting with Windows 2000, when trying to overwrite an already
         existing file with FILE_ATTRIBUTE_HIDDEN and/or FILE_ATTRIBUTE_SYSTEM
@@ -626,7 +626,7 @@ fhandler_base::open (int flags, mode_t mode)
     }
 
   status = NtCreateFile (&fh, access, &attr, &io, NULL, file_attributes, shared,
-                        create_disposition, create_options, p, plen);
+                        create_disposition, options, p, plen);
   if (!NT_SUCCESS (status))
     {
       /* Trying to create a directory should return EISDIR, not ENOENT. */
@@ -672,7 +672,7 @@ done:
   debug_printf ("%x = NtCreateFile "
                "(%p, %x, %S, io, NULL, %x, %x, %x, %x, NULL, 0)",
                status, fh, access, pc.get_nt_native_path (), file_attributes,
-               shared, create_disposition, create_options);
+               shared, create_disposition, options);
 
   syscall_printf ("%d = fhandler_base::open (%S, %p)",
                  res, pc.get_nt_native_path (), flags);
index 8268a9a..5120a3c 100644 (file)
@@ -152,7 +152,9 @@ class fhandler_base
   } status, open_status;
 
  private:
-  int access;
+  ACCESS_MASK access;
+  ULONG options;
+
   HANDLE io_handle;
 
   __ino64_t ino;       /* file ID or hashed filename, depends on FS. */
@@ -206,8 +208,11 @@ class fhandler_base
   DWORD get_minor () { return dev ().get_minor (); }
   virtual int get_unit () { return dev ().get_minor (); }
 
-  int get_access () const { return access; }
-  void set_access (int x) { access = x; }
+  ACCESS_MASK get_access () const { return access; }
+  void set_access (ACCESS_MASK x) { access = x; }
+
+  ULONG get_options () const { return options; }
+  void set_options (ULONG x) { options = x; }
 
   int get_flags () { return openflags; }
   void set_flags (int x, int supplied_bin = 0);
@@ -801,13 +806,19 @@ class fhandler_dev_tape: public fhandler_dev_raw
 
 class fhandler_disk_file: public fhandler_base
 {
+  HANDLE prw_handle;
   int readdir_helper (DIR *, dirent *, DWORD, DWORD, PUNICODE_STRING fname) __attribute__ ((regparm (3)));
 
+  int prw_open (bool);
+
  public:
   fhandler_disk_file ();
   fhandler_disk_file (path_conv &pc);
 
   int open (int flags, mode_t mode);
+  int close ();
+  int dup (fhandler_base *child);
+  void fixup_after_fork (HANDLE parent);
   int lock (int, struct __flock64 *);
   bool isdevice () const { return false; }
   int __stdcall fstat (struct __stat64 *buf) __attribute__ ((regparm (2)));
index a57427b..9cebbe4 100644 (file)
@@ -1356,12 +1356,12 @@ fhandler_base::utimens_fs (const struct timespec *tvp)
 }
 
 fhandler_disk_file::fhandler_disk_file () :
-  fhandler_base ()
+  fhandler_base (), prw_handle (NULL)
 {
 }
 
 fhandler_disk_file::fhandler_disk_file (path_conv &pc) :
-  fhandler_base ()
+  fhandler_base (), prw_handle (NULL)
 {
   set_name (pc);
 }
@@ -1373,6 +1373,35 @@ fhandler_disk_file::open (int flags, mode_t mode)
 }
 
 int
+fhandler_disk_file::close ()
+{
+  if (prw_handle)
+    NtClose (prw_handle);
+  return fhandler_base::close ();
+}
+
+int
+fhandler_disk_file::dup (fhandler_base *child)
+{
+  fhandler_disk_file *fhc = (fhandler_disk_file *) child;
+
+  int ret = fhandler_base::dup (child);
+  if (!ret && prw_handle
+      && !DuplicateHandle (GetCurrentProcess (), prw_handle,
+                          GetCurrentProcess (), &fhc->prw_handle,
+                          0, TRUE, DUPLICATE_SAME_ACCESS))
+    prw_handle = NULL;
+  return ret;
+}
+
+void
+fhandler_disk_file::fixup_after_fork (HANDLE parent)
+{
+  prw_handle = NULL;
+  fhandler_base::fixup_after_fork (parent);
+}
+
+int
 fhandler_base::open_fs (int flags, mode_t mode)
 {
   /* Unfortunately NT allows to open directories for writing, but that's
@@ -1412,6 +1441,73 @@ out:
   return res;
 }
 
+/* POSIX demands that pread/pwrite don't change the current file position.
+   While NtReadFile/NtWriteFile support atomic seek-and-io, both change the
+   file pointer if the file handle has been opened for synchonous I/O.
+   Using this handle for pread/pwrite would break atomicity, because the
+   read/write operation would have to be followed by a seek back to the old
+   file position.  What we do is to open another handle to the file on the
+   first call to either pread or pwrite.  This is used for any subsequent
+   pread/pwrite.  Thus the current file position of the "normal" file
+   handle is not touched.
+   
+   FIXME:
+
+   Note that this is just a hack.  The problem with this approach is that
+   a change to the file permissions might disallow to open the file with
+   the required permissions to read or write.  This appears to be a border case,
+   but that's exactly what git does.  It creates the file for reading and
+   writing and after writing it, it chmods the file to read-only.  Then it
+   calls pread on the file to examine the content.  This works, but if git
+   would use the original handle to pwrite to the file, it would be broken
+   with our approach.
+
+   One way to implement this is to open the pread/pwrite handle right at
+   file open time.  We would simply maintain two handles, which wouldn't
+   be much of a problem given how we do that for other fhandler types as
+   well.
+
+   However, ultimately fhandler_disk_file should become a derived class of
+   fhandler_base_overlapped.  Each raw_read or raw_write would fetch the
+   actual file position, read/write from there, and then set the file
+   position again.  Fortunately, while the file position is not maintained
+   by the I/O manager, it can be fetched and set to a new value by all
+   processes holding a handle to that file object.  Pread and pwrite differ
+   from raw_read and raw_write just by not touching the current file pos.
+   Actually they could be merged with raw_read/raw_write if we add a position
+   parameter to the latter. */
+
+int
+fhandler_disk_file::prw_open (bool write)
+{
+  NTSTATUS status;
+  IO_STATUS_BLOCK io;
+  OBJECT_ATTRIBUTES attr;
+
+  /* First try to open with the original access mask */
+  ACCESS_MASK access = get_access ();
+  pc.init_reopen_attr (&attr, get_handle ());
+  status = NtOpenFile (&prw_handle, access, &attr, &io,
+                      FILE_SHARE_VALID_FLAGS, get_options ());
+  if (status == STATUS_ACCESS_DENIED)
+    {
+      /* If we get an access denied, chmod has been called.  Try again
+        with just the required rights to perform the called function. */
+      access &= write ? ~GENERIC_READ : ~GENERIC_WRITE;
+      status = NtOpenFile (&prw_handle, access, &attr, &io,
+                          FILE_SHARE_VALID_FLAGS, get_options ());
+    }
+  debug_printf ("%x = NtOpenFile (%p, %x, %S, io, %x, %x)",
+               status, prw_handle, access, pc.get_nt_native_path (),
+               FILE_SHARE_VALID_FLAGS, get_options ());
+  if (!NT_SUCCESS (status))
+    {
+      __seterrno_from_nt_status (status);
+      return -1;
+    }
+  return 0;
+}
+
 ssize_t __stdcall
 fhandler_disk_file::pread (void *buf, size_t count, _off64_t offset)
 {
@@ -1429,7 +1525,9 @@ fhandler_disk_file::pread (void *buf, size_t count, _off64_t offset)
       IO_STATUS_BLOCK io;
       LARGE_INTEGER off = { QuadPart:offset };
 
-      status = NtReadFile (get_handle (), NULL, NULL, NULL, &io, buf, count,
+      if (!prw_handle && prw_open (false))
+       goto non_atomic;
+      status = NtReadFile (prw_handle, NULL, NULL, NULL, &io, buf, count,
                           &off, NULL);
       if (!NT_SUCCESS (status))
        {
@@ -1440,15 +1538,15 @@ fhandler_disk_file::pread (void *buf, size_t count, _off64_t offset)
            }
          if (status == (NTSTATUS) STATUS_ACCESS_VIOLATION)
            {
-             if (is_at_eof (get_handle ()))
+             if (is_at_eof (prw_handle))
                return 0;
              switch (mmap_is_attached_or_noreserve (buf, count))
                {
                case MMAP_NORESERVE_COMMITED:
-                 status = NtReadFile (get_handle (), NULL, NULL, NULL, &io,
+                 status = NtReadFile (prw_handle, NULL, NULL, NULL, &io,
                                       buf, count, &off, NULL);
                  if (NT_SUCCESS (status))
-                   goto success;
+                   return io.Information;
                  break;
                case MMAP_RAISE_SIGBUS:
                  raise (SIGBUS);
@@ -1459,12 +1557,9 @@ fhandler_disk_file::pread (void *buf, size_t count, _off64_t offset)
          __seterrno_from_nt_status (status);
          return -1;
        }
-
-success:
-      lseek (-io.Information, SEEK_CUR);
-      return io.Information;
     }
 
+non_atomic:
   /* Text mode stays slow and non-atomic. */
   ssize_t res;
   _off64_t curpos = lseek (0, SEEK_CUR);
@@ -1499,7 +1594,9 @@ fhandler_disk_file::pwrite (void *buf, size_t count, _off64_t offset)
       IO_STATUS_BLOCK io;
       LARGE_INTEGER off = { QuadPart:offset };
 
-      status = NtWriteFile (get_handle (), NULL, NULL, NULL, &io, buf, count,
+      if (!prw_handle && prw_open (true))
+       goto non_atomic;
+      status = NtWriteFile (prw_handle, NULL, NULL, NULL, &io, buf, count,
                            &off, NULL);
       if (!NT_SUCCESS (status))
        {
@@ -1508,6 +1605,8 @@ fhandler_disk_file::pwrite (void *buf, size_t count, _off64_t offset)
        }
       return io.Information;
     }
+
+non_atomic:
   /* Text mode stays slow and non-atomic. */
   int res;
   _off64_t curpos = lseek (0, SEEK_CUR);