To support the new BT daemon, we need to add some improvements to the socket IPC classes.

Comment on attachment 8505406 [details] [diff] [review] [01] Bug 1083092: Introduce |mozilla::ipc::SocketBase| Everyone's on PTO...

Comment on attachment 8505406 [details] [diff] [review] [01] Bug 1083092: Introduce |mozilla::ipc::SocketBase| Review of attachment 8505406 [details] [diff] [review]: ----------------------------------------------------------------- This looks fine to me.

Comment on attachment 8505407 [details] [diff] [review] [02] Bug 1083092: Data parameter for |mozilla::ipc::SocketIOSendTask| template Review of attachment 8505407 [details] [diff] [review]: ----------------------------------------------------------------- The change makes sense. I'm fine with this change, although I'm not the best person to review UnixSocket.

Comment on attachment 8505409 [details] [diff] [review] [03] Bug 1083092: Add |mozilla::ipc::UnixSocketIOBuffer| Review of attachment 8505409 [details] [diff] [review]: ----------------------------------------------------------------- ::: ipc/unixsocket/SocketBase.cpp @@ +57,5 @@ > + const uint8_t* data = Consume(aLen); > + if (!data) { > + return NS_ERROR_OUT_OF_MEMORY; > + } > + memcpy(aValue, data, aLen); I will be better to do sanity check for| memcpy| @@ +67,5 @@ > +{ > + if (((mAvailableSpace - mSize) < aLen)) { > + size_t availableSpace = mAvailableSpace + std::max(mAvailableSpace, aLen); > + uint8_t* data = new uint8_t[availableSpace]; > + memcpy(data, mData, mSize); I will be better to do sanity check for| memcpy|

Hi, thanks for your review. (In reply to Shawn Huang [:shawnjohnjr] from comment #7) > Comment on attachment 8505409 [details] [diff] [review] > [03] Bug 1083092: Add |mozilla::ipc::UnixSocketIOBuffer| > > Review of attachment 8505409 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: ipc/unixsocket/SocketBase.cpp > @@ +57,5 @@ > > + const uint8_t* data = Consume(aLen); > > + if (!data) { > > + return NS_ERROR_OUT_OF_MEMORY; > > + } > > + memcpy(aValue, data, aLen); > > I will be better to do sanity check for| memcpy| What do you mean by 'sanity check'?

I was thinking if we need to check dest buffer size is big enough. But I was wrong, we don't need to do it.

Thank you, Shawn. https://hg.mozilla.org/integration/b2g-inbound/rev/6fe61837183d https://hg.mozilla.org/integration/b2g-inbound/rev/8a3c29ab6c2c https://hg.mozilla.org/integration/b2g-inbound/rev/c8edfa5e934c https://treeherder.mozilla.org/ui/#/jobs?repo=b2g-inbound&revision=c8edfa5e934c

https://hg.mozilla.org/mozilla-central/rev/6fe61837183d https://hg.mozilla.org/mozilla-central/rev/8a3c29ab6c2c https://hg.mozilla.org/mozilla-central/rev/c8edfa5e934c

https://hg.mozilla.org/mozilla-central/rev/6fe61837183d https://hg.mozilla.org/mozilla-central/rev/8a3c29ab6c2c https://hg.mozilla.org/mozilla-central/rev/c8edfa5e934c