Bug 931720 - Return low-level error correctly from nsLocalFile::CopyToNative(). r=bsmedberg
authorISHIKAWA, Chiaki <ishikawa@yk.rim.or.jp>
Thu, 31 Oct 2013 23:04:11 -0400
changeset 152981 ed35c291d57ed85b5394775e163aeb8d96f6b1ee
parent 152980 eac93c93b680d48726a0231b5b17daa6b378cf61
child 152982 13bd430455d43197995da6bc99d0f5c0f1c5dfbc
push id25564
push useremorley@mozilla.com
push dateFri, 01 Nov 2013 13:13:59 +0000
treeherdermozilla-central@5297b280ebeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbsmedberg
bugs931720
milestone28.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 931720 - Return low-level error correctly from nsLocalFile::CopyToNative(). r=bsmedberg Fix copytonative to return better meaningful error (from errno) when PR_Write()/PR_Read() failed (e.g., failed to write to full file system, etc.)
xpcom/io/nsLocalFileUnix.cpp
--- a/xpcom/io/nsLocalFileUnix.cpp
+++ b/xpcom/io/nsLocalFileUnix.cpp
@@ -812,46 +812,80 @@ nsLocalFile::CopyToNative(nsIFile *newPa
 
 #ifdef DEBUG_blizzard
         int32_t totalRead = 0;
         int32_t totalWritten = 0;
 #endif
         char buf[BUFSIZ];
         int32_t bytesRead;
         
+        // record PR_Write() error for better error message later.
+        nsresult saved_write_error = NS_OK;    
+        nsresult saved_read_error = NS_OK;
+
+        // DONE: Does PR_Read() return bytesRead < 0 for error?
+        // Yes., The errors from PR_Read are not so common and
+        // the value may not have correspondence in NS_ERROR_*, but
+        // we do catch it still, immediately after while() loop.
+        // We can differentiate errors pf PR_Read and PR_Write by
+        // looking at saved_write_error value. If PR_Write error occurs (and not
+        // PR_Read() error), save_write_error is not NS_OK.
+
         while ((bytesRead = PR_Read(oldFD, buf, BUFSIZ)) > 0) {
 #ifdef DEBUG_blizzard
             totalRead += bytesRead;
 #endif
 
             // PR_Write promises never to do a short write
             int32_t bytesWritten = PR_Write(newFD, buf, bytesRead);
             if (bytesWritten < 0) {
+                saved_write_error = NSRESULT_FOR_ERRNO();
                 bytesRead = -1;
                 break;
             }
             NS_ASSERTION(bytesWritten == bytesRead, "short PR_Write?");
 
 #ifdef DEBUG_blizzard
             totalWritten += bytesWritten;
 #endif
         }
 
+        // Record error if PR_Read() failed.
+        // Must be done before any other I/O which may reset errno.
+        if ( (bytesRead < 0) && (saved_write_error == NS_OK)) {
+            saved_read_error = NSRESULT_FOR_ERRNO();
+        }
+
 #ifdef DEBUG_blizzard
         printf("read %d bytes, wrote %d bytes\n",
                  totalRead, totalWritten);
 #endif
 
+        // TODO/FIXME: better find out how to propagate errors of
+        // close.  Errors of close can occur.  Read man page of
+        // close(2); At least, we should tell the user that
+        // filesystem/disk is hosed so that users can take remedial
+        // action.
+
         // close the files
         PR_Close(newFD);
         PR_Close(oldFD);
 
-        // check for read (or write) error after cleaning up
-        if (bytesRead < 0) 
-            return NS_ERROR_OUT_OF_MEMORY;
+        // Let us report the failure to write and read.
+        // check for write/read error after cleaning up
+        if (bytesRead < 0) {
+            if (saved_write_error != NS_OK)
+                return saved_write_error;
+            else if (saved_read_error != NS_OK)
+                return saved_read_error;
+#if DEBUG
+            else                // sanity check. Die and debug.
+                MOZ_ASSERT(0);
+#endif
+        }
     }
     return rv;
 }
 
 NS_IMETHODIMP
 nsLocalFile::CopyToFollowingLinksNative(nsIFile *newParent, const nsACString &newName)
 {
     return CopyToNative(newParent, newName);