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

Unified Diff: runtime/bin/process.cc

Issue 1194883002: Improve the encoding/decoding to/from system encoding on Windows (Closed) Base URL: https://github.com/dart-lang/sdk.git@master
Patch Set: A few more comments Created 5 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | runtime/bin/utils.h » ('j') | runtime/bin/utils.h » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: runtime/bin/process.cc
diff --git a/runtime/bin/process.cc b/runtime/bin/process.cc
index 88ace0e95913842e103fc0e82fbf2adb884bed59..05bb83402fc705205919e9a9e7f5101f9e081667 100644
--- a/runtime/bin/process.cc
+++ b/runtime/bin/process.cc
@@ -298,21 +298,49 @@ void FUNCTION_NAME(SystemEncodingToString)(Dart_NativeArguments args) {
delete[] buffer;
Dart_PropagateError(result);
}
+ intptr_t len;
char* str =
- StringUtils::ConsoleStringToUtf8(reinterpret_cast<char*>(buffer));
- Dart_SetReturnValue(args, DartUtils::NewString(str));
+ StringUtils::ConsoleStringToUtf8(
+ reinterpret_cast<char*>(buffer),
+ bytes_length,
+ &len);
+ result =
+ Dart_NewStringFromUTF8(reinterpret_cast<const uint8_t*>(str), len);
+ // On some platforms StringUtils::Utf8ToConsoleString is a no op, and
Ivan Posva 2015/06/22 08:54:16 Comment does not match the call here?
Søren Gjesse 2015/06/23 11:17:59 Done.
+ // the data to convert is passed through. In that case don't free the
+ // result of the conversion as nothing is allocated.
if (str != reinterpret_cast<char*>(buffer)) free(str);
+ delete[] buffer;
Ivan Posva 2015/06/22 08:54:16 Wouldn't it be easier and safer to use Dart_ScopeA
Søren Gjesse 2015/06/23 11:17:59 Changed it for the buffer variable used here. We c
+ if (Dart_IsError(result)) {
+ Dart_PropagateError(result);
+ }
+ Dart_SetReturnValue(args, result);
}
void FUNCTION_NAME(StringToSystemEncoding)(Dart_NativeArguments args) {
Dart_Handle str = Dart_GetNativeArgument(args, 0);
- const char* utf8 = DartUtils::GetStringValue(str);
- const char* system_string = StringUtils::Utf8ToConsoleString(utf8);
- int external_length = strlen(system_string);
+ char* utf8;
+ intptr_t utf8_len;
+ Dart_Handle result = Dart_StringToUTF8(
+ str, reinterpret_cast<uint8_t **>(&utf8), &utf8_len);
+ if (Dart_IsError(result)) {
+ Dart_PropagateError(result);
+ }
+ intptr_t system_len;
+ const char* system_string =
+ StringUtils::Utf8ToConsoleString(utf8, utf8_len, &system_len);
uint8_t* buffer = NULL;
- Dart_Handle external_array = IOBuffer::Allocate(external_length, &buffer);
- memmove(buffer, system_string, external_length);
+ Dart_Handle external_array = IOBuffer::Allocate(system_len, &buffer);
Lasse Reichstein Nielsen 2015/06/22 12:08:34 Could Utf8ToConsoleString be made to allocate usin
Søren Gjesse 2015/06/23 11:17:58 I don't want Utf8ToConsoleString to allocate in a
+ if (Dart_IsError(external_array)) {
+ // See comment further down.
Lasse Reichstein Nielsen 2015/06/22 12:08:34 Put comment here, say "See comment above for why t
Søren Gjesse 2015/06/23 11:17:58 Done.
+ if (utf8 != system_string) free(const_cast<char*>(system_string));
+ Dart_PropagateError(result);
+ }
+ memmove(buffer, system_string, system_len);
+ // On some platforms StringUtils::Utf8ToConsoleString is a no op, and
+ // the data to convert is passed through. In that case don't free the
+ // result of the conversion as nothing is allocated.
Lasse Reichstein Nielsen 2015/06/22 12:08:34 as nothing new has been allocated.
Søren Gjesse 2015/06/23 11:17:59 Done.
if (utf8 != system_string) free(const_cast<char*>(system_string));
Dart_SetReturnValue(args, external_array);
}
« no previous file with comments | « no previous file | runtime/bin/utils.h » ('j') | runtime/bin/utils.h » ('J')

Powered by Google App Engine
This is Rietveld 408576698