Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(534)

Issue 10441078: Add test that passes invalid arguments to socket methods and fix (Closed)

Created:
8 years, 6 months ago by Mads Ager (google)
Modified:
8 years, 6 months ago
Reviewers:
Søren Gjesse
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Add test that passes invalid arguments to socket methods and fix issues encountered. The fuzzing is not as systematic as for some of the other APIs. However, a lot of checking is going on in the Dart code so forcing issues required a bit of work making objects act like lists and integers. Additionally, fix a memory leak in process implementation. R=sgjesse@google.com BUG= TEST= Committed: https://code.google.com/p/dart/source/detail?r=8088

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+154 lines, -57 lines) Patch
M runtime/bin/process.cc View 1 chunk +1 line, -0 lines 0 comments Download
M runtime/bin/socket.cc View 6 chunks +80 lines, -56 lines 2 comments Download
M runtime/bin/socket_impl.dart View 1 chunk +1 line, -1 line 0 comments Download
A tests/standalone/io/socket_invalid_arguments_test.dart View 1 chunk +71 lines, -0 lines 0 comments Download
M tests/standalone/standalone.status View 1 chunk +1 line, -0 lines 2 comments Download

Messages

Total messages: 3 (0 generated)
Mads Ager (google)
8 years, 6 months ago (2012-05-29 18:54:26 UTC) #1
Søren Gjesse
lgtm https://chromiumcodereview.appspot.com/10441078/diff/1/runtime/bin/socket.cc File runtime/bin/socket.cc (right): https://chromiumcodereview.appspot.com/10441078/diff/1/runtime/bin/socket.cc#newcode233 runtime/bin/socket.cc:233: const char* bind_address = DartUtils::GetStringValue(bind_address_obj); We could consider ...
8 years, 6 months ago (2012-05-30 07:07:15 UTC) #2
Mads Ager (google)
8 years, 6 months ago (2012-05-30 07:35:11 UTC) #3
https://chromiumcodereview.appspot.com/10441078/diff/1/runtime/bin/socket.cc
File runtime/bin/socket.cc (right):

https://chromiumcodereview.appspot.com/10441078/diff/1/runtime/bin/socket.cc#...
runtime/bin/socket.cc:233: const char* bind_address =
DartUtils::GetStringValue(bind_address_obj);
On 2012/05/30 07:07:15, Søren Gjesse wrote:
> We could consider having GetStringValue work like GetInt64Value.

Yes, that could be done. It makes sense to have two version. One that asserts
and one that checks. I'll see if I can streamline that in an upcoming change.

https://chromiumcodereview.appspot.com/10441078/diff/1/tests/standalone/stand...
File tests/standalone/standalone.status (right):

https://chromiumcodereview.appspot.com/10441078/diff/1/tests/standalone/stand...
tests/standalone/standalone.status:18: io/socket_invalid_arguments_test: Fail,
OK
On 2012/05/30 07:07:15, Søren Gjesse wrote:
> I am still confused by the state "Fail, OK" :-|

It is failing and that is OK. :-)

What it means is that the test fails and that the failure is not something that
we want to fix. In the summary report (using the --report option to the test
scripts) of failing tests this is listed as a failure that we don't want to fix.

Powered by Google App Engine
This is Rietveld 408576698