Bug 1005323: In nsTemporaryFileInputStream::ReadSegments, call writer correctly. r=roc
authorJim Blandy <jimb@mozilla.com>
Thu, 03 Jul 2014 11:32:50 -0700
changeset 213039 e4738365e0a1ab4b3d375d9a45f94ab5eb26d712
parent 213038 ed419a60029cd64fdedf2d40cb27047b0ffe8f38
child 213040 05e8235c3e20db065bd6ec7723c6ab262961e6fa
push id3857
push userraliiev@mozilla.com
push dateTue, 02 Sep 2014 16:39:23 +0000
treeherdermozilla-beta@5638b907b505 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs1005323
milestone33.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 1005323: In nsTemporaryFileInputStream::ReadSegments, call writer correctly. r=roc - Respect the byte count that writer returns; don't assume it always accepts the full amount. - If writer returns an error, return NS_OK with a partial write count; don't continue writing data. - Simplify counters slightly.
netwerk/base/src/nsTemporaryFileInputStream.cpp
--- a/netwerk/base/src/nsTemporaryFileInputStream.cpp
+++ b/netwerk/base/src/nsTemporaryFileInputStream.cpp
@@ -1,9 +1,9 @@
-/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsTemporaryFileInputStream.h"
 #include "nsStreamUtils.h"
 #include <algorithm>
 
@@ -55,36 +55,49 @@ nsTemporaryFileInputStream::ReadSegments
 
   if (mClosed) {
     return NS_BASE_STREAM_CLOSED;
   }
 
   mozilla::MutexAutoLock lock(mFileDescOwner->FileMutex());
   PR_Seek64(mFileDescOwner->mFD, mStartPos, PR_SEEK_SET);
 
+  // Limit requested count to the amount remaining in our section of the file.
   count = std::min(count, uint32_t(mEndPos - mStartPos));
-  uint32_t remainBufCount = count;
 
   char buf[4096];
-  while (remainBufCount > 0) {
-    uint32_t bufCount = std::min(remainBufCount, (uint32_t)sizeof(buf));
-    int32_t read_result = PR_Read(mFileDescOwner->mFD, buf, bufCount);
-    if (read_result < 0) {
+  while (*result < count) {
+    uint32_t bufCount = std::min(count - *result, (uint32_t) sizeof(buf));
+    int32_t bytesRead = PR_Read(mFileDescOwner->mFD, buf, bufCount);
+    if (bytesRead < 0) {
       return NS_ErrorAccordingToNSPR();
     }
-    uint32_t write_result = 0;
-    nsresult rv = writer(this, closure, buf,
-                         count - remainBufCount, bufCount, &write_result);
-    remainBufCount -= bufCount;
-    NS_ENSURE_SUCCESS(rv, rv);
-    NS_ASSERTION(write_result <= bufCount,
-                 "writer should not write more than we asked it to write");
-    mStartPos += bufCount;
+
+    int32_t bytesWritten = 0;
+    while (bytesWritten < bytesRead) {
+      uint32_t writerCount = 0;
+      nsresult rv = writer(this, closure, buf + bytesWritten, *result,
+                           bytesRead - bytesWritten, &writerCount);
+      if (NS_FAILED(rv) || writerCount == 0) {
+        // nsIInputStream::ReadSegments' contract specifies that errors
+        // from writer are not propagated to ReadSegments' caller.
+        //
+        // If writer fails, leaving bytes still in buf, that's okay: we
+        // only update mStartPos to reflect successful writes, so the call
+        // to PR_Seek64 at the top will restart us at the right spot.
+        return NS_OK;
+      }
+      NS_ASSERTION(writerCount <= (uint32_t) (bytesRead - bytesWritten),
+                   "writer should not write more than we asked it to write");
+      bytesWritten += writerCount;
+      *result += writerCount;
+      mStartPos += writerCount;
+    }
   }
-  *result = count;
+
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsTemporaryFileInputStream::IsNonBlocking(bool * nonBlocking)
 {
   *nonBlocking = false;
   return NS_OK;