Mojo: DataPipe: Write some docs and rationalize some return values.

mojo/system/local_data_pipe.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // TODO(vtl): I currently potentially overflow in doing index calculations.
 // E.g., |start_index_| and |current_num_bytes_| fit into a |uint32_t|, but
 // their sum may not. This is bad and poses a security risk. (We're currently
 // saved by the limit on capacity -- the maximum size of the buffer, checked in
 // |DataPipe::ValidateOptions()|, is currently sufficiently small.
 
@@ skipping 32 matching lines @@
     DestroyBufferNoLock();
   }
   AwakeConsumerWaitersForStateChangeNoLock();
 }
 
 MojoResult LocalDataPipe::ProducerWriteDataImplNoLock(const void* elements,
                                                       uint32_t* num_bytes,
                                                       bool all_or_none) {
   DCHECK_EQ(*num_bytes % element_num_bytes(), 0u);
   DCHECK_GT(*num_bytes, 0u);
+  DCHECK(consumer_open_no_lock());
 
   size_t num_bytes_to_write = 0;
   if (may_discard()) {
     if (all_or_none && *num_bytes > capacity_num_bytes())
       return MOJO_RESULT_OUT_OF_RANGE;
 
     num_bytes_to_write = std::min(static_cast<size_t>(*num_bytes),
                                   capacity_num_bytes());
     if (num_bytes_to_write > capacity_num_bytes() - current_num_bytes_) {
       // Discard as much as needed (discard oldest first).
       size_t num_bytes_to_discard =
           num_bytes_to_write - (capacity_num_bytes() - current_num_bytes_);
       start_index_ += num_bytes_to_discard;
       start_index_ %= capacity_num_bytes();
       current_num_bytes_ -= num_bytes_to_discard;
     }
   } else {
     if (all_or_none && *num_bytes > capacity_num_bytes() - current_num_bytes_) {
-      return (*num_bytes > capacity_num_bytes()) ? MOJO_RESULT_OUT_OF_RANGE :
-                                                   MOJO_RESULT_SHOULD_WAIT;
+      // Don't return "should wait" since you can't wait for a specified amount
+      // of data.
+      return MOJO_RESULT_OUT_OF_RANGE;
     }
 
     num_bytes_to_write = std::min(static_cast<size_t>(*num_bytes),
                                   capacity_num_bytes() - current_num_bytes_);
   }
   if (num_bytes_to_write == 0)
     return MOJO_RESULT_SHOULD_WAIT;
 
   // The amount we can write in our first |memcpy()|.
   size_t num_bytes_to_write_first =
@@ skipping 20 matching lines @@
     AwakeConsumerWaitersForStateChangeNoLock();
 
   *num_bytes = static_cast<uint32_t>(num_bytes_to_write);
   return MOJO_RESULT_OK;
 }
 
 MojoResult LocalDataPipe::ProducerBeginWriteDataImplNoLock(
     void** buffer,
     uint32_t* buffer_num_bytes,
     bool all_or_none) {
+  DCHECK(consumer_open_no_lock());
+
   size_t max_num_bytes_to_write = GetMaxNumBytesToWriteNoLock();
-  // TODO(vtl): Consider this return value.
-  if (all_or_none && *buffer_num_bytes > max_num_bytes_to_write)
+  if (all_or_none && *buffer_num_bytes > max_num_bytes_to_write) {
+    // Don't return "should wait" since you can't wait for a specified amount of
+    // data.
     return MOJO_RESULT_OUT_OF_RANGE;
+  }
 
   // Don't go into a two-phase write if there's no room.
   if (max_num_bytes_to_write == 0)
     return MOJO_RESULT_SHOULD_WAIT;
 
   size_t write_index =
       (start_index_ + current_num_bytes_) % capacity_num_bytes();
   EnsureBufferNoLock();
   *buffer = buffer_.get() + write_index;
   *buffer_num_bytes = static_cast<uint32_t>(max_num_bytes_to_write);
@@ skipping 45 matching lines @@
   current_num_bytes_ = 0;
   AwakeProducerWaitersForStateChangeNoLock();
 }
 
 MojoResult LocalDataPipe::ConsumerReadDataImplNoLock(void* elements,
                                                      uint32_t* num_bytes,
                                                      bool all_or_none) {
   DCHECK_EQ(*num_bytes % element_num_bytes(), 0u);
   DCHECK_GT(*num_bytes, 0u);
 
-  // TODO(vtl): Think about the error code in this case.
-  if (all_or_none && *num_bytes > current_num_bytes_)
-    return MOJO_RESULT_OUT_OF_RANGE;
+  if (all_or_none && *num_bytes > current_num_bytes_) {
+    // Don't return "should wait" since you can't wait for a specified amount of
+    // data.
+    return producer_open_no_lock() ? MOJO_RESULT_OUT_OF_RANGE :
+                                     MOJO_RESULT_FAILED_PRECONDITION;
+  }
 
   size_t num_bytes_to_read =
       std::min(static_cast<size_t>(*num_bytes), current_num_bytes_);
   if (num_bytes_to_read == 0) {
     return producer_open_no_lock() ? MOJO_RESULT_SHOULD_WAIT :
                                      MOJO_RESULT_FAILED_PRECONDITION;
   }
 
   // The amount we can read in our first |memcpy()|.
   size_t num_bytes_to_read_first =
@@ skipping 18 matching lines @@
 
   *num_bytes = static_cast<uint32_t>(num_bytes_to_read);
   return MOJO_RESULT_OK;
 }
 
 MojoResult LocalDataPipe::ConsumerDiscardDataImplNoLock(uint32_t* num_bytes,
                                                         bool all_or_none) {
   DCHECK_EQ(*num_bytes % element_num_bytes(), 0u);
   DCHECK_GT(*num_bytes, 0u);
 
-  // TODO(vtl): Think about the error code in this case.
-  if (all_or_none && *num_bytes > current_num_bytes_)
-    return MOJO_RESULT_OUT_OF_RANGE;
+  if (all_or_none && *num_bytes > current_num_bytes_) {
+    // Don't return "should wait" since you can't wait for a specified amount of
+    // data.
+    return producer_open_no_lock() ? MOJO_RESULT_OUT_OF_RANGE :
+                                     MOJO_RESULT_FAILED_PRECONDITION;
+  }
 
   // Be consistent with other operations; error if no data available.
   if (current_num_bytes_ == 0) {
     return producer_open_no_lock() ? MOJO_RESULT_SHOULD_WAIT :
                                      MOJO_RESULT_FAILED_PRECONDITION;
   }
 
   bool was_full = (current_num_bytes_ == capacity_num_bytes());
 
   size_t num_bytes_to_discard =
@@ skipping 12 matching lines @@
   // Note: This cast is safe, since the capacity fits into a |uint32_t|.
   *num_bytes = static_cast<uint32_t>(current_num_bytes_);
   return MOJO_RESULT_OK;
 }
 
 MojoResult LocalDataPipe::ConsumerBeginReadDataImplNoLock(
     const void** buffer,
     uint32_t* buffer_num_bytes,
     bool all_or_none) {
   size_t max_num_bytes_to_read = GetMaxNumBytesToReadNoLock();
-  // TODO(vtl): Consider this return value.
-  if (all_or_none && *buffer_num_bytes > max_num_bytes_to_read)
+  if (all_or_none && *buffer_num_bytes > max_num_bytes_to_read) {
+    // Don't return "should wait" since you can't wait for a specified amount of
+    // data.
     return MOJO_RESULT_OUT_OF_RANGE;
+  }
 
   // Don't go into a two-phase read if there's no data.
   if (max_num_bytes_to_read == 0) {
     return producer_open_no_lock() ? MOJO_RESULT_SHOULD_WAIT :
                                      MOJO_RESULT_FAILED_PRECONDITION;
   }
 
   *buffer = buffer_.get() + start_index_;
   *buffer_num_bytes = static_cast<uint32_t>(max_num_bytes_to_read);
   set_consumer_two_phase_max_num_bytes_read_no_lock(
@@ skipping 69 matching lines @@
 }
 
 size_t LocalDataPipe::GetMaxNumBytesToReadNoLock() {
   if (start_index_ + current_num_bytes_ > capacity_num_bytes())
     return capacity_num_bytes() - start_index_;
   return current_num_bytes_;
 }
 
 }  // namespace system
 }  // namespace mojo