IO v2: Handle illegal arguments to socket writes

IO v2: Handle illegal arguments to socket writes

Anders and Mads: Is this the right way to propagate the consumer error
to the pipe future?

Ivan: Please comment on the throwing of an ArgumentError from
dart_api_impl.c

R=ajohnsen@google.com, ager@google.com, iposva@google.com
Committed: https://code.google.com/p/dart/source/detail?r=17896

[Mads Ager (google), 2013-01-31 07:53:30]

LGTM with the TODO in dart_api_impl restored.

https://codereview.chromium.org/12089071/diff/3001/runtime/bin/socket_patch.dart
File runtime/bin/socket_patch.dart (right):

https://codereview.chromium.org/12089071/diff/3001/runtime/bin/socket_patch.d...
runtime/bin/socket_patch.dart:697: offset += socket._write(buffer, offset,
buffer.length - offset);
Accidental indentation change?

https://codereview.chromium.org/12089071/diff/3001/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/12089071/diff/3001/runtime/vm/dart_api_impl.c...
runtime/vm/dart_api_impl.cc:2053: ASSERT(integer.AsInt64Value() <= 0xff);       
                          \
I think this is what the TODO was about? It seems easy to hit this ASSERT. We
should either document the truncating behavior and remove the assert or make
this check part of the argument error check above.

Related question: what happens if the integer is a bigint that does not fit in
an int64_t?

For now, maybe we should leave the error handling TODO in there listing these
issues?

https://codereview.chromium.org/12089071/diff/3001/tests/standalone/io/socket...
File tests/standalone/io/socket_invalid_arguments_test.dart (right):

https://codereview.chromium.org/12089071/diff/3001/tests/standalone/io/socket...
tests/standalone/io/socket_invalid_arguments_test.dart:30: .then((socket) {
I would move this to the line above and remove some indentation from the body.

https://codereview.chromium.org/12089071/diff/3001/tests/standalone/io/socket...
tests/standalone/io/socket_invalid_arguments_test.dart:66: // TODO(ajohnsen):
the following two fails.
Is this still the case?

[Søren Gjesse, 2013-01-31 12:12:01]

Landing with the tests requiring throwing from Dart_ListGetAsBytes disables as
the throwing currently fails in debug mode. Opened bug
https://code.google.com/p/dart/issues/detail?id=8233.

https://codereview.chromium.org/12089071/diff/3001/runtime/bin/socket_patch.dart
File runtime/bin/socket_patch.dart (right):

https://codereview.chromium.org/12089071/diff/3001/runtime/bin/socket_patch.d...
runtime/bin/socket_patch.dart:697: offset += socket._write(buffer, offset,
buffer.length - offset);
On 2013/01/31 07:53:30, Mads Ager wrote:
> Accidental indentation change?

Done.

https://codereview.chromium.org/12089071/diff/3001/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/12089071/diff/3001/runtime/vm/dart_api_impl.c...
runtime/vm/dart_api_impl.cc:2053: ASSERT(integer.AsInt64Value() <= 0xff);       
                          \
On 2013/01/31 07:53:30, Mads Ager wrote:
> I think this is what the TODO was about? It seems easy to hit this ASSERT. We
> should either document the truncating behavior and remove the assert or make
> this check part of the argument error check above.
> 
> Related question: what happens if the integer is a bigint that does not fit in
> an int64_t?
> 
> For now, maybe we should leave the error handling TODO in there listing these
> issues?

You are right. I just saw the clamping and thought that the TODO was no longer
valid. Added it back in.

https://codereview.chromium.org/12089071/diff/3001/tests/standalone/io/socket...
File tests/standalone/io/socket_invalid_arguments_test.dart (right):

https://codereview.chromium.org/12089071/diff/3001/tests/standalone/io/socket...
tests/standalone/io/socket_invalid_arguments_test.dart:30: .then((socket) {
On 2013/01/31 07:53:30, Mads Ager wrote:
> I would move this to the line above and remove some indentation from the body.

Done.

https://codereview.chromium.org/12089071/diff/3001/tests/standalone/io/socket...
tests/standalone/io/socket_invalid_arguments_test.dart:66: // TODO(ajohnsen):
the following two fails.
On 2013/01/31 07:53:30, Mads Ager wrote:
> Is this still the case?

It is still the case (I even added more with negative numbers and bigints which
also fails).