Bug 1276590: Replace |ReadBytes| with |ReadBytesInto| in Gonk IPC code, r=froydnj,billm draft
authorThomas Zimmermann <tdz@users.sourceforge.net>
Mon, 06 Jun 2016 12:18:12 +0200
changeset 375625 99a0db3cac4e482987d52029dc98b1055f031ebb
parent 375369 3e8ee3599a67edd971770af4982ad4b0fe77f073
child 522919 332e7caee2838c61c9634f406752f3605ccf40e2
push id20331
push usertdz@users.sourceforge.net
push dateMon, 06 Jun 2016 10:20:06 +0000
reviewersfroydnj, billm
bugs1276590
milestone49.0a1
Bug 1276590: Replace |ReadBytes| with |ReadBytesInto| in Gonk IPC code, r=froydnj,billm MozReview-Commit-ID: FdjdoOT7j7j
gfx/layers/ipc/GonkNativeHandleUtils.cpp
gfx/layers/ipc/ShadowLayerUtilsGralloc.cpp
--- a/gfx/layers/ipc/GonkNativeHandleUtils.cpp
+++ b/gfx/layers/ipc/GonkNativeHandleUtils.cpp
@@ -1,21 +1,37 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*-
  * vim: sw=2 ts=8 et :
  */
 /* 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 "GonkNativeHandleUtils.h"
+#include "mozilla/UniquePtr.h"
+#include "mozilla/unused.h"
 
 using namespace mozilla::layers;
 
 namespace IPC {
 
+namespace {
+
+class native_handle_Delete
+{
+public:
+  void operator()(native_handle* aNativeHandle) const
+  {
+    native_handle_close(aNativeHandle); // closes file descriptors
+    native_handle_delete(aNativeHandle);
+  }
+};
+
+} // anonymous namespace
+
 void
 ParamTraits<GonkNativeHandle>::Write(Message* aMsg,
                                 const paramType& aParam)
 {
   GonkNativeHandle handle = aParam;
   MOZ_ASSERT(handle.IsValid());
 
   RefPtr<GonkNativeHandle::NhObj> nhObj = handle.GetAndResetNhObj();
@@ -30,42 +46,48 @@ ParamTraits<GonkNativeHandle>::Write(Mes
   }
 }
 
 bool
 ParamTraits<GonkNativeHandle>::Read(const Message* aMsg,
                                PickleIterator* aIter, paramType* aResult)
 {
   size_t nbytes;
-  const char* data;
-  if (!aMsg->ReadSize(aIter, &nbytes) ||
-      !aMsg->ReadBytes(aIter, &data, nbytes)) {
+  if (!aMsg->ReadSize(aIter, &nbytes)) {
     return false;
   }
 
   if (nbytes % sizeof(int) != 0) {
     return false;
   }
 
   size_t numInts = nbytes / sizeof(int);
   size_t numFds = aMsg->num_fds();
-  native_handle* nativeHandle = native_handle_create(numFds, numInts);
+  mozilla::UniquePtr<native_handle, native_handle_Delete> nativeHandle(
+    native_handle_create(numFds, numInts));
   if (!nativeHandle) {
     return false;
   }
 
-  memcpy(nativeHandle->data + nativeHandle->numFds, data, nbytes);
+  auto data =
+    reinterpret_cast<char*>(nativeHandle->data + nativeHandle->numFds);
+  if (!aMsg->ReadBytesInto(aIter, data, nbytes)) {
+    return false;
+  }
 
-  for (size_t i = 0; i < static_cast<size_t>(nativeHandle->numFds); ++i) {
+  for (size_t i = 0; i < numFds; ++i) {
     base::FileDescriptor fd;
     if (!aMsg->ReadFileDescriptor(aIter, &fd)) {
       return false;
     }
     nativeHandle->data[i] = fd.fd;
+    nativeHandle->numFds = i + 1; // set number of valid file descriptors
   }
 
-  GonkNativeHandle handle(new GonkNativeHandle::NhObj(nativeHandle));
+  GonkNativeHandle handle(new GonkNativeHandle::NhObj(nativeHandle.get()));
   handle.TransferToAnother(*aResult);
 
+  mozilla::Unused << nativeHandle.release();
+
   return true;
 }
 
 } // namespace IPC
--- a/gfx/layers/ipc/ShadowLayerUtilsGralloc.cpp
+++ b/gfx/layers/ipc/ShadowLayerUtilsGralloc.cpp
@@ -10,16 +10,17 @@
 #include "mozilla/gfx/Point.h"
 #include "mozilla/layers/LayerTransactionChild.h"
 #include "mozilla/layers/ShadowLayers.h"
 #include "mozilla/layers/LayerManagerComposite.h"
 #include "mozilla/layers/CompositorTypes.h"
 #include "mozilla/layers/TextureHost.h"
 #include "mozilla/layers/SharedBufferManagerChild.h"
 #include "mozilla/layers/SharedBufferManagerParent.h"
+#include "mozilla/UniquePtr.h"
 #include "mozilla/unused.h"
 #include "nsXULAppAPI.h"
 
 #include "ShadowLayerUtilsGralloc.h"
 
 #include "nsIMemoryReporter.h"
 
 #include "gfxPlatform.h"
@@ -111,65 +112,70 @@ bool
 ParamTraits<MagicGrallocBufferHandle>::Read(const Message* aMsg,
                                             PickleIterator* aIter, paramType* aResult)
 {
   MOZ_ASSERT(!aResult->mGraphicBuffer.get());
   MOZ_ASSERT(aResult->mRef.mOwner == 0);
   MOZ_ASSERT(aResult->mRef.mKey == -1);
 
   size_t nbytes;
-  const char* data;
   int owner;
   int64_t index;
 
   if (!aMsg->ReadInt(aIter, &owner) ||
       !aMsg->ReadInt64(aIter, &index) ||
-      !aMsg->ReadSize(aIter, &nbytes) ||
-      !aMsg->ReadBytes(aIter, &data, nbytes)) {
+      !aMsg->ReadSize(aIter, &nbytes)) {
+    printf_stderr("ParamTraits<MagicGrallocBufferHandle>::Read() failed to read a message\n");
+    return false;
+  }
+
+  auto data = mozilla::MakeUnique<char[]>(nbytes);
+
+  if (!aMsg->ReadBytesInto(aIter, data.get(), nbytes)) {
     printf_stderr("ParamTraits<MagicGrallocBufferHandle>::Read() failed to read a message\n");
     return false;
   }
 
   size_t nfds = aMsg->num_fds();
   int fds[nfds];
 
   for (size_t n = 0; n < nfds; ++n) {
     FileDescriptor fd;
     if (!aMsg->ReadFileDescriptor(aIter, &fd)) {
       printf_stderr("ParamTraits<MagicGrallocBufferHandle>::Read() failed to read file descriptors\n");
       return false;
     }
     // If the GraphicBuffer was shared cross-process, SCM_RIGHTS does
     // the right thing and dup's the fd. If it's shared cross-thread,
-    // SCM_RIGHTS doesn't dup the fd. 
+    // SCM_RIGHTS doesn't dup the fd.
     // But in shared cross-thread, dup fd is not necessary because we get
     // a pointer to the GraphicBuffer directly from SharedBufferManagerParent
     // and don't create a new GraphicBuffer around the fd.
     fds[n] = fd.fd;
   }
 
   aResult->mRef.mOwner = owner;
   aResult->mRef.mKey = index;
   if (XRE_IsParentProcess()) {
     // If we are in chrome process, we can just get GraphicBuffer directly from
     // SharedBufferManagerParent.
     aResult->mGraphicBuffer = SharedBufferManagerParent::GetGraphicBuffer(aResult->mRef);
   } else {
     // Deserialize GraphicBuffer
 #if ANDROID_VERSION >= 19
     sp<GraphicBuffer> buffer(new GraphicBuffer());
-    const void* datap = (const void*)data;
+    const void* datap = (const void*)data.get();
     const int* fdsp = &fds[0];
     if (NO_ERROR != buffer->unflatten(datap, nbytes, fdsp, nfds)) {
       buffer = nullptr;
     }
 #else
     sp<GraphicBuffer> buffer(new GraphicBuffer());
     Flattenable *flattenable = buffer.get();
-    if (NO_ERROR != flattenable->unflatten(data, nbytes, fds, nfds)) {
+    if (NO_ERROR != flattenable->unflatten(data.get(), nbytes, fds, nfds)) {
       buffer = nullptr;
     }
 #endif
     if (buffer.get()) {
       aResult->mGraphicBuffer = buffer;
     }
   }