Start using Dart_PropagateError in the runtime/bin directory.

Start using Dart_PropagateError in the runtime/bin directory.
Committed: https://code.google.com/p/dart/source/detail?r=5197

[Søren Gjesse, 2012-03-08 21:18:46]

lgtm

the ASSERTS where worse, but we really need to think about how to get the C++
cleanup right. I have not checked all the call sites for wich there now might
not be a return anymore if an error occurs, but I should be very surprised if
there are not some with pending C++ cleanup.

http://codereview.chromium.org/9657001/diff/6002/runtime/bin/dartutils.h
File runtime/bin/dartutils.h (right):

http://codereview.chromium.org/9657001/diff/6002/runtime/bin/dartutils.h#newc...
runtime/bin/dartutils.h:62: // these functions.
I can see how Dart_PropagateError makes it simpler to handle errors. However
this comment states why it is also error prone. This will lead to difficult to
find errors, so I am not sure I like it - of cause it is better than the ASSERTS
it is replacing.

Could it be an idea to have some kind of propagate scope to set up

{
  PropagateErrorScope s;
  // Call functions which use Dart_PropagateError
  ...
}

This will only be a marker at the call site that calls might never return.

Of cause all the functions below here should then check for this scope being
entered.

Not sure this realy helps though.

http://codereview.chromium.org/9657001/diff/6002/runtime/bin/file.cc
File runtime/bin/file.cc (right):

http://codereview.chromium.org/9657001/diff/6002/runtime/bin/file.cc#newcode195
runtime/bin/file.cc:195: if (Dart_IsError(result)) {
Missing delete[] buffer.

http://codereview.chromium.org/9657001/diff/6002/runtime/bin/file.cc#newcode228
runtime/bin/file.cc:228: if (Dart_IsError(result)) {
Missing delete[] buffer.

http://codereview.chromium.org/9657001/diff/6002/runtime/bin/process.cc
File runtime/bin/process.cc (right):

http://codereview.chromium.org/9657001/diff/6002/runtime/bin/process.cc#newco...
runtime/bin/process.cc:43: if (Dart_IsError(arg)) {
Missing delete[] string_args.

http://codereview.chromium.org/9657001/diff/6002/runtime/bin/socket.cc
File runtime/bin/socket.cc (right):

http://codereview.chromium.org/9657001/diff/6002/runtime/bin/socket.cc#newcode65
runtime/bin/socket.cc:65: if (Dart_IsError(result)) {
Missing delete[] buffer

http://codereview.chromium.org/9657001/diff/6002/runtime/bin/socket.cc#newcod...
runtime/bin/socket.cc:110: if (Dart_IsError(result)) {
Missing delete[] buffer.

[turnidge, 2012-03-08 22:19:26]

http://codereview.chromium.org/9657001/diff/6002/runtime/bin/dartutils.h
File runtime/bin/dartutils.h (right):

http://codereview.chromium.org/9657001/diff/6002/runtime/bin/dartutils.h#newc...
runtime/bin/dartutils.h:62: // these functions.
On 2012/03/08 21:18:46, Søren Gjesse wrote:
> I can see how Dart_PropagateError makes it simpler to handle errors. However
> this comment states why it is also error prone. This will lead to difficult to
> find errors, so I am not sure I like it - of cause it is better than the
ASSERTS
> it is replacing.
> 
> Could it be an idea to have some kind of propagate scope to set up
> 
> {
>   PropagateErrorScope s;
>   // Call functions which use Dart_PropagateError
>   ...
> }
> 
> This will only be a marker at the call site that calls might never return.
> 
> Of cause all the functions below here should then check for this scope being
> entered.
> 
> Not sure this realy helps though. 

From offline discussion.

I will change the api to DartUtils to be less error prone in a follow-up CL. 
For the time being, I will leave the ASSERTs inside of DartUtils.

http://codereview.chromium.org/9657001/diff/6002/runtime/bin/file.cc
File runtime/bin/file.cc (right):

http://codereview.chromium.org/9657001/diff/6002/runtime/bin/file.cc#newcode195
runtime/bin/file.cc:195: if (Dart_IsError(result)) {
On 2012/03/08 21:18:46, Søren Gjesse wrote:
> Missing delete[] buffer.

Done.

http://codereview.chromium.org/9657001/diff/6002/runtime/bin/file.cc#newcode228
runtime/bin/file.cc:228: if (Dart_IsError(result)) {
On 2012/03/08 21:18:46, Søren Gjesse wrote:
> Missing delete[] buffer.

Done.

http://codereview.chromium.org/9657001/diff/6002/runtime/bin/process.cc
File runtime/bin/process.cc (right):

http://codereview.chromium.org/9657001/diff/6002/runtime/bin/process.cc#newco...
runtime/bin/process.cc:43: if (Dart_IsError(arg)) {
On 2012/03/08 21:18:46, Søren Gjesse wrote:
> Missing delete[] string_args.

Done.

[Søren Gjesse, 2012-03-08 22:51:42]

lgtm