Fixes some memory leaks in //runtime/bin

Fixes some memory leaks in //runtime/bin

Also some style cleanups.

Decided to fix these leaks by using Dart_ScopeAllocate
instead of malloc and new. Leaks are noted in the CL.

I haven't finished looking over all the code in
//runtime/bin yet, but this CL was getting big.

Committed: https://github.com/dart-lang/sdk/commit/c75a817ee0f307f50ffbe5f9e64653173d1e26ef

[zra, 2016-03-11 23:54:00]

Description was changed from

==========
Fixes memory leaks in //runtime/bin
==========

to

==========
Fixes some memory leaks in //runtime/bin

Decided to fix these leaks by using Dart_ScopeAllocate
instead of malloc and new. Leaks are noted in the CL.

I haven't finished looking over all the code in
//runtime/bin yet, but this CL was getting big.
==========

[zra, 2016-03-11 23:55:03]

Description was changed from

==========
Fixes some memory leaks in //runtime/bin

Decided to fix these leaks by using Dart_ScopeAllocate
instead of malloc and new. Leaks are noted in the CL.

I haven't finished looking over all the code in
//runtime/bin yet, but this CL was getting big.
==========

to

==========
Fixes some memory leaks in //runtime/bin

Also some style cleanups.

Decided to fix these leaks by using Dart_ScopeAllocate
instead of malloc and new. Leaks are noted in the CL.

I haven't finished looking over all the code in
//runtime/bin yet, but this CL was getting big.
==========

[zra, 2016-03-11 23:55:49]

zra@google.com changed reviewers:
+ asiva@google.com, iposva@google.com

[zra, 2016-03-11 23:55:51]

https://codereview.chromium.org/1781883002/diff/160001/runtime/bin/directory_...
File runtime/bin/directory_win.cc (left):

https://codereview.chromium.org/1781883002/diff/160001/runtime/bin/directory_...
runtime/bin/directory_win.cc:467: if (exists != EXISTS) return false;
leaked system_path and system_new_path.

https://codereview.chromium.org/1781883002/diff/160001/runtime/bin/directory_...
runtime/bin/directory_win.cc:474: if (!success) return false;
ditto

https://codereview.chromium.org/1781883002/diff/160001/runtime/bin/file_macos.cc
File runtime/bin/file_macos.cc (left):

https://codereview.chromium.org/1781883002/diff/160001/runtime/bin/file_macos...
runtime/bin/file_macos.cc:370: char* resolved_path =
reinterpret_cast<char*>(malloc(PATH_MAX + 1));
leaks resolved_path (never gets passed to realpath).

https://codereview.chromium.org/1781883002/diff/160001/runtime/bin/file_win.cc
File runtime/bin/file_win.cc (left):

https://codereview.chromium.org/1781883002/diff/160001/runtime/bin/file_win.c...
runtime/bin/file_win.cc:323: if (CloseHandle(dir_handle) == 0) return false;
leaks target and data_buffer.

https://codereview.chromium.org/1781883002/diff/180001/runtime/bin/process.cc
File runtime/bin/process.cc (left):

https://codereview.chromium.org/1781883002/diff/180001/runtime/bin/process.cc...
runtime/bin/process.cc:60: Dart_PropagateError(result);
leaks string_args.

https://codereview.chromium.org/1781883002/diff/180001/runtime/bin/process.cc...
runtime/bin/process.cc:65: Dart_PropagateError(result);
ditto

https://codereview.chromium.org/1781883002/diff/180001/runtime/bin/process.cc...
runtime/bin/process.cc:137: ExtractCStringList(environment,
leaks string_args if it calls Dart_PropagateError.

https://codereview.chromium.org/1781883002/diff/180001/runtime/bin/process.cc...
runtime/bin/process.cc:171: Socket::SetSocketIdNativeField(stdin_handle,
process_stdin);
These may call Dart_PropagateError. If they do they leak string_args,
string_environment, and os_error_message

https://codereview.chromium.org/1781883002/diff/180001/runtime/bin/process.cc...
runtime/bin/process.cc:183: Dart_PropagateError(result);
leaks string_args, string_environment, and os_error_message.

[Ivan Posva, 2016-03-13 20:59:19]

LGTMwC -ip

https://codereview.chromium.org/1781883002/diff/200001/runtime/bin/dartutils.h
File runtime/bin/dartutils.h (right):

https://codereview.chromium.org/1781883002/diff/200001/runtime/bin/dartutils....
runtime/bin/dartutils.h:180: strncpy(result, str, str_len);
Could you use memcpy or memmove instead? There is no reason to be checking for
the nul-character in the loop in the C library function.

https://codereview.chromium.org/1781883002/diff/200001/runtime/bin/directory_...
File runtime/bin/directory_android.cc (right):

https://codereview.chromium.org/1781883002/diff/200001/runtime/bin/directory_...
runtime/bin/directory_android.cc:26: data_ = calloc(PATH_MAX + 1, sizeof(char));
 // NOLINT
Please add a comment where this is being free'd.

https://codereview.chromium.org/1781883002/diff/200001/runtime/bin/directory_...
File runtime/bin/directory_linux.cc (right):

https://codereview.chromium.org/1781883002/diff/200001/runtime/bin/directory_...
runtime/bin/directory_linux.cc:373: char* result =
DartUtils::ScopedCString(PATH_MAX);
Did you consider not having to over allocate from the scoped memory area
smilarly to CurrentNoScope()?

[zra, 2016-03-14 18:07:52]

https://codereview.chromium.org/1781883002/diff/200001/runtime/bin/dartutils.h
File runtime/bin/dartutils.h (right):

https://codereview.chromium.org/1781883002/diff/200001/runtime/bin/dartutils....
runtime/bin/dartutils.h:180: strncpy(result, str, str_len);
On 2016/03/13 20:59:18, Ivan Posva wrote:
> Could you use memcpy or memmove instead? There is no reason to be checking for
> the nul-character in the loop in the C library function.

Done.

https://codereview.chromium.org/1781883002/diff/200001/runtime/bin/directory_...
File runtime/bin/directory_android.cc (right):

https://codereview.chromium.org/1781883002/diff/200001/runtime/bin/directory_...
runtime/bin/directory_android.cc:26: data_ = calloc(PATH_MAX + 1, sizeof(char));
 // NOLINT
On 2016/03/13 20:59:19, Ivan Posva wrote:
> Please add a comment where this is being free'd.

Moved the destructor from the header file into the source files so that the free
is right next to the calloc.

https://codereview.chromium.org/1781883002/diff/200001/runtime/bin/directory_...
File runtime/bin/directory_linux.cc (right):

https://codereview.chromium.org/1781883002/diff/200001/runtime/bin/directory_...
runtime/bin/directory_linux.cc:373: char* result =
DartUtils::ScopedCString(PATH_MAX);
On 2016/03/13 20:59:19, Ivan Posva wrote:
> Did you consider not having to over allocate from the scoped memory area
> smilarly to CurrentNoScope()?

Changed to be like CurrentNoScope but with ScopedCopyCString instead of strdup.

[zra, 2016-03-14 18:08:55]

Description was changed from

==========
Fixes some memory leaks in //runtime/bin

Also some style cleanups.

Decided to fix these leaks by using Dart_ScopeAllocate
instead of malloc and new. Leaks are noted in the CL.

I haven't finished looking over all the code in
//runtime/bin yet, but this CL was getting big.
==========

to

==========
Fixes some memory leaks in //runtime/bin

Also some style cleanups.

Decided to fix these leaks by using Dart_ScopeAllocate
instead of malloc and new. Leaks are noted in the CL.

I haven't finished looking over all the code in
//runtime/bin yet, but this CL was getting big.

Committed:
https://github.com/dart-lang/sdk/commit/c75a817ee0f307f50ffbe5f9e64653173d1e26ef
==========

[zra, 2016-03-14 18:08:56]

Committed patchset #13 (id:240001) manually as
c75a817ee0f307f50ffbe5f9e64653173d1e26ef (presubmit successful).