Fix recursive directory deletion, to not leak file descriptors (and pointers).

Fix recursive directory deletion, to not leak file descriptors (and pointers).

BUG=
R=sgjesse@google.com

Committed: https://code.google.com/p/dart/source/detail?r=35310

[Søren Gjesse, 2014-04-23 11:58:46]

lgtm, with comments. Comments for Linux version goes for all platforms.

https://codereview.chromium.org/249233002/diff/1/runtime/bin/directory_linux.cc
File runtime/bin/directory_linux.cc (right):

https://codereview.chromium.org/249233002/diff/1/runtime/bin/directory_linux....
runtime/bin/directory_linux.cc:248: DIR* dir_pointer =
opendir(path->AsString());
NO_RETRY_EXPECTED here?

https://codereview.chromium.org/249233002/diff/1/runtime/bin/directory_linux....
runtime/bin/directory_linux.cc:260: return
NO_RETRY_EXPECTED(closedir(dir_pointer)) == 0 &&
closedir is using VOID_NO_RETRY_EXPECTED below

https://codereview.chromium.org/249233002/diff/1/runtime/bin/directory_linux....
runtime/bin/directory_linux.cc:304: }
Add comment that we only get here if there is an error. Maybe assert that errno
is not 0.

[Anders Johnsen, 2014-04-23 12:09:03]

https://codereview.chromium.org/249233002/diff/1/runtime/bin/directory_linux.cc
File runtime/bin/directory_linux.cc (right):

https://codereview.chromium.org/249233002/diff/1/runtime/bin/directory_linux....
runtime/bin/directory_linux.cc:248: DIR* dir_pointer =
opendir(path->AsString());
On 2014/04/23 11:58:47, Søren Gjesse wrote:
> NO_RETRY_EXPECTED here?

Not compatible with pointers, sadly.

https://codereview.chromium.org/249233002/diff/1/runtime/bin/directory_linux....
runtime/bin/directory_linux.cc:260: return
NO_RETRY_EXPECTED(closedir(dir_pointer)) == 0 &&
On 2014/04/23 11:58:47, Søren Gjesse wrote:
> closedir is using VOID_NO_RETRY_EXPECTED below

Yes, that's because we already failed in that case (we can't double-report
errors).

https://codereview.chromium.org/249233002/diff/1/runtime/bin/directory_linux....
runtime/bin/directory_linux.cc:304: }
On 2014/04/23 11:58:47, Søren Gjesse wrote:
> Add comment that we only get here if there is an error. Maybe assert that
errno
> is not 0.

Done.

[Anders Johnsen, 2014-04-23 12:09:26]

Committed patchset #2 manually as r35310 (presubmit successful).