Review commit: https://reviewboard.mozilla.org/r/56550/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/56550/

Comment on attachment 8758245 [details] Bug 1276590: Replace |ReadBytes| with |ReadBytesInto| in Gonk IPC code, https://reviewboard.mozilla.org/r/56550/#review53102 r=me with the changes below, assuming they pass tests. ::: gfx/layers/ipc/GonkNativeHandleUtils.cpp:40 (Diff revision 1) > PickleIterator* aIter, paramType* aResult) > { > size_t nbytes; > const char* data; > if (!aMsg->ReadSize(aIter, &nbytes) || > - !aMsg->ReadBytes(aIter, &data, nbytes)) { > + !aMsg->ReadBytesInto(aIter, &data, nbytes)) { ReadBytesInto assumes that its second parameter points to a chunk of memory that is `nbytes` long, which isn't the case here. I think you want: aMsg->FlattenBytes(aIter, &data, nbytes) instead. ::: gfx/layers/ipc/ShadowLayerUtilsGralloc.cpp:126 (Diff revision 1) > int64_t index; > > if (!aMsg->ReadInt(aIter, &owner) || > !aMsg->ReadInt64(aIter, &index) || > !aMsg->ReadSize(aIter, &nbytes) || > - !aMsg->ReadBytes(aIter, &data, nbytes)) { > + !aMsg->ReadBytesInto(aIter, &data, nbytes)) { Same thing here.

Pushed by tdz@users.sourceforge.net: https://hg.mozilla.org/integration/mozilla-inbound/rev/1f80fae4ca27 Replace |ReadBytes| with |ReadBytesInto| in Gonk IPC code, r=froydnj

backed out by request from Thomas

Backout by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/69312b20d716 Backed out changeset 1f80fae4ca27 on developers request

We're going to be removing FlattenBytes in a month or so (once bug 1264642 is done). I wouldn't recommend using it. Instead, it would be better to use ReadBytesInto to read into actual allocated memory.

Comment on attachment 8758245 [details] Bug 1276590: Replace |ReadBytes| with |ReadBytesInto| in Gonk IPC code, Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56550/diff/1-2/

Comment on attachment 8758245 [details] Bug 1276590: Replace |ReadBytes| with |ReadBytesInto| in Gonk IPC code, https://reviewboard.mozilla.org/r/56550/#review53720 ::: gfx/layers/ipc/GonkNativeHandleUtils.cpp:56 (Diff revision 2) > } > > - memcpy(nativeHandle->data + nativeHandle->numFds, data, nbytes); > + auto data = > + reinterpret_cast<char*>(nativeHandle->data + nativeHandle->numFds); > + if (!aMsg->ReadBytesInto(aIter, data, nbytes)) { > + return false; If this fails, it looks like you'll leak nativeHandle.

https://reviewboard.mozilla.org/r/56550/#review53720 > If this fails, it looks like you'll leak nativeHandle. This method has always been leaking on errors. I'll fix that with |UniquePtr| in the next iteration of the patch.

Comment on attachment 8758245 [details] Bug 1276590: Replace |ReadBytes| with |ReadBytesInto| in Gonk IPC code, Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56550/diff/2-3/

Pushed by tdz@users.sourceforge.net: https://hg.mozilla.org/integration/mozilla-inbound/rev/e9b2e7b412b9 Replace |ReadBytes| with |ReadBytesInto| in Gonk IPC code, r=froydnj,billm

Thank you! https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=e9b2e7b412b9