nacl_io: Add SOCK_DGRAM support to socketpair()

nacl_io: Add SOCK_DGRAM support to socketpair()

Change UnixEventEmitter to use a generic FIFOInterface instead of
FIFOChar specifically.  Add bytewise Read() and Write() methods to
the former interface so that subclasses can handle both stream and
datagram based communication.  For SOCK_DGRAM, use the FIFOPacket
class as the backend instead of FIFOChar.

BUG=584925
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_nacl_sdk;tryserver.chromium.mac:mac_nacl_sdk;tryserver.chromium.win:win_nacl_sdk

Committed: https://crrev.com/bbf1e34ff094449db2c1d738067cd6ffbaf66134
Cr-Commit-Position: refs/heads/master@{#374248}

[Kevin Cernekee, 2016-02-07 22:16:55]

cernekee@chromium.org changed reviewers:
+ binji@chromium.org, sbc@chromium.org

[Sam Clegg, 2016-02-08 17:30:56]

Thanks for working on this!

Looks good with a new nits.

https://codereview.chromium.org/1679643002/diff/40001/native_client_sdk/src/l...
File native_client_sdk/src/libraries/nacl_io/socket/unix_event_emitter.h
(right):

https://codereview.chromium.org/1679643002/diff/40001/native_client_sdk/src/l...
native_client_sdk/src/libraries/nacl_io/socket/unix_event_emitter.h:42: virtual
FIFOInterface* out_fifoc() = 0;
Can we just drop in_fifoc and out_fifoc now?

https://codereview.chromium.org/1679643002/diff/40001/native_client_sdk/src/l...
File native_client_sdk/src/libraries/nacl_io/socket/unix_node.h (right):

https://codereview.chromium.org/1679643002/diff/40001/native_client_sdk/src/l...
native_client_sdk/src/libraries/nacl_io/socket/unix_node.h:19: explicit
UnixNode(Filesystem* filesystem, int type);
Drop the explicit?

https://codereview.chromium.org/1679643002/diff/40001/native_client_sdk/src/t...
File native_client_sdk/src/tests/nacl_io_test/socket_test.cc (right):

https://codereview.chromium.org/1679643002/diff/40001/native_client_sdk/src/t...
native_client_sdk/src/tests/nacl_io_test/socket_test.cc:362: char outbuf1[256],
outbuf2[128];
nit: put these one on each line

https://codereview.chromium.org/1679643002/diff/40001/native_client_sdk/src/t...
native_client_sdk/src/tests/nacl_io_test/socket_test.cc:405: // blocks when
there is no more data to read.
Didn't you already send data in the opposite direction just above?  Maybe just
attempt to recv again here.  That would also verify that the remainder of the
packet (above the 16 bytes you received) was discarded.

[cernekee, 2016-02-08 22:53:28]

cernekee@google.com changed reviewers:
+ cernekee@google.com

[cernekee, 2016-02-08 22:53:29]

https://codereview.chromium.org/1679643002/diff/40001/native_client_sdk/src/l...
File native_client_sdk/src/libraries/nacl_io/socket/unix_event_emitter.h
(right):

https://codereview.chromium.org/1679643002/diff/40001/native_client_sdk/src/l...
native_client_sdk/src/libraries/nacl_io/socket/unix_event_emitter.h:42: virtual
FIFOInterface* out_fifoc() = 0;
On 2016/02/08 17:30:55, Sam Clegg wrote:
> Can we just drop in_fifoc and out_fifoc now?

Done.

https://codereview.chromium.org/1679643002/diff/40001/native_client_sdk/src/l...
File native_client_sdk/src/libraries/nacl_io/socket/unix_node.h (right):

https://codereview.chromium.org/1679643002/diff/40001/native_client_sdk/src/l...
native_client_sdk/src/libraries/nacl_io/socket/unix_node.h:19: explicit
UnixNode(Filesystem* filesystem, int type);
On 2016/02/08 17:30:55, Sam Clegg wrote:
> Drop the explicit?

Done.

https://codereview.chromium.org/1679643002/diff/40001/native_client_sdk/src/t...
File native_client_sdk/src/tests/nacl_io_test/socket_test.cc (right):

https://codereview.chromium.org/1679643002/diff/40001/native_client_sdk/src/t...
native_client_sdk/src/tests/nacl_io_test/socket_test.cc:362: char outbuf1[256],
outbuf2[128];
On 2016/02/08 17:30:55, Sam Clegg wrote:
> nit: put these one on each line

Done.

https://codereview.chromium.org/1679643002/diff/40001/native_client_sdk/src/t...
native_client_sdk/src/tests/nacl_io_test/socket_test.cc:405: // blocks when
there is no more data to read.
On 2016/02/08 17:30:55, Sam Clegg wrote:
> Didn't you already send data in the opposite direction just above?  Maybe just
> attempt to recv again here.  That would also verify that the remainder of the
> packet (above the 16 bytes you received) was discarded.

Done.

[Sam Clegg, 2016-02-08 23:12:27]

Great.  LGTM.

Can you run this through the CQ with
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_nacl_sdk;tryserver.chromium.mac:mac_nacl_sdk;tryserver.chromium.win:win_nacl_sdk

[Kevin Cernekee, 2016-02-08 23:30:20]

Description was changed from

==========
nacl_io: Add SOCK_DGRAM support to socketpair()

Change UnixEventEmitter to use a generic FIFOInterface instead of
FIFOChar specifically.  Add bytewise Read() and Write() methods to
the former interface so that subclasses can handle both stream and
datagram based communication.  For SOCK_DGRAM, use the FIFOPacket
class as the backend instead of FIFOChar.

BUG=584925
==========

to

==========
nacl_io: Add SOCK_DGRAM support to socketpair()

Change UnixEventEmitter to use a generic FIFOInterface instead of
FIFOChar specifically.  Add bytewise Read() and Write() methods to
the former interface so that subclasses can handle both stream and
datagram based communication.  For SOCK_DGRAM, use the FIFOPacket
class as the backend instead of FIFOChar.

BUG=584925
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_nacl_sdk;tryserver.chromium.mac:mac_nacl_sdk;tryserver.chromium.win:win_nacl_sdk
==========

[cernekee, 2016-02-08 23:30:40]

The CQ bit was checked by cernekee@google.com

[commit-bot: I haz the power, 2016-02-08 23:34:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1679643002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1679643002/60001

[commit-bot: I haz the power, 2016-02-09 01:16:28]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-02-09 01:18:08]

Description was changed from

==========
nacl_io: Add SOCK_DGRAM support to socketpair()

Change UnixEventEmitter to use a generic FIFOInterface instead of
FIFOChar specifically.  Add bytewise Read() and Write() methods to
the former interface so that subclasses can handle both stream and
datagram based communication.  For SOCK_DGRAM, use the FIFOPacket
class as the backend instead of FIFOChar.

BUG=584925
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_nacl_sdk;tryserver.chromium.mac:mac_nacl_sdk;tryserver.chromium.win:win_nacl_sdk
==========

to

==========
nacl_io: Add SOCK_DGRAM support to socketpair()

Change UnixEventEmitter to use a generic FIFOInterface instead of
FIFOChar specifically.  Add bytewise Read() and Write() methods to
the former interface so that subclasses can handle both stream and
datagram based communication.  For SOCK_DGRAM, use the FIFOPacket
class as the backend instead of FIFOChar.

BUG=584925
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_nacl_sdk;tryserver.chromium.mac:mac_nacl_sdk;tryserver.chromium.win:win_nacl_sdk

Committed: https://crrev.com/bbf1e34ff094449db2c1d738067cd6ffbaf66134
Cr-Commit-Position: refs/heads/master@{#374248}
==========

[commit-bot: I haz the power, 2016-02-09 01:18:09]

Patchset 4 (id:??) landed as
https://crrev.com/bbf1e34ff094449db2c1d738067cd6ffbaf66134
Cr-Commit-Position: refs/heads/master@{#374248}