Issue 11558012: Use FormatMessageW for Windows error messages to handle internationalized messages correctly.

Issue 11558012: Use FormatMessageW for Windows error messages to handle internationalized messages correctly. (Closed)

Created:
8 years ago by Mads Ager (google)

Modified:
8 years ago

Reviewers:
Søren Gjesse

CC:
reviews_dartlang.org

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

Use FormatMessageW for Windows error messages to handle internationalized messages correctly. R=sgjesse@google.com BUG=dartbug.com/6986 Committed: https://code.google.com/p/dart/source/detail?r=16041

Patch Set 2 : Share some code. #

Total comments: 2

Patch Set 3 : Fix new/free mismatch. #

Total comments: 2

Patch Set 4 : Address comments. #

Created: 8 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+270 lines, -327 lines)			Patch
M	runtime/bin/directory_win.cc	View	1	11 chunks	+12 lines, -34 lines	0 comments	Download
M	runtime/bin/file_win.cc	View	1	10 chunks	+10 lines, -10 lines	0 comments	Download
M	runtime/bin/main.cc	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	runtime/bin/platform.h	View		1 chunk	+0 lines, -4 lines	0 comments	Download
M	runtime/bin/platform_android.cc	View		1 chunk	+0 lines, -9 lines	0 comments	Download
M	runtime/bin/platform_linux.cc	View		1 chunk	+0 lines, -13 lines	0 comments	Download
M	runtime/bin/platform_macos.cc	View		1 chunk	+0 lines, -9 lines	0 comments	Download
M	runtime/bin/platform_win.cc	View	1	3 chunks	+1 line, -40 lines	0 comments	Download
M	runtime/bin/process.h	View		1 chunk	+1 line, -2 lines	0 comments	Download
M	runtime/bin/process.cc	View	1	5 chunks	+5 lines, -5 lines	0 comments	Download
M	runtime/bin/process_android.cc	View		8 chunks	+25 lines, -31 lines	0 comments	Download
M	runtime/bin/process_linux.cc	View		8 chunks	+25 lines, -31 lines	0 comments	Download
M	runtime/bin/process_macos.cc	View		8 chunks	+25 lines, -31 lines	0 comments	Download
M	runtime/bin/process_win.cc	View	1	12 chunks	+31 lines, -29 lines	0 comments	Download
M	runtime/bin/utils.h	View	1	1 chunk	+8 lines, -4 lines	0 comments	Download
M	runtime/bin/utils_android.cc	View	1	1 chunk	+24 lines, -4 lines	0 comments	Download
M	runtime/bin/utils_linux.cc	View	1	1 chunk	+24 lines, -4 lines	0 comments	Download
M	runtime/bin/utils_macos.cc	View	1	1 chunk	+24 lines, -4 lines	0 comments	Download
M	runtime/bin/utils_win.cc	View	1 2 3	3 chunks	+54 lines, -30 lines	0 comments	Download
M	tests/standalone/io/directory_error_test.dart	View		5 chunks	+0 lines, -32 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

Søren Gjesse

lgtm LGTM! https://codereview.chromium.org/11558012/diff/3001/runtime/bin/utils_win.cc File runtime/bin/utils_win.cc (right): https://codereview.chromium.org/11558012/diff/3001/runtime/bin/utils_win.cc#newcode39 runtime/bin/utils_win.cc:39: delete utf8; delete[] - more below. https://codereview.chromium.org/11558012/diff/16/runtime/bin/process.cc ...

8 years ago (2012-12-12 14:00:38 UTC) #3

Mads Ager (google)

8 years ago (2012-12-12 14:37:59 UTC) #4

Thank you!

https://codereview.chromium.org/11558012/diff/3001/runtime/bin/utils_win.cc
File runtime/bin/utils_win.cc (right):

https://codereview.chromium.org/11558012/diff/3001/runtime/bin/utils_win.cc#n...
runtime/bin/utils_win.cc:39: delete utf8;
On 2012/12/12 14:00:38, Søren Gjesse wrote:
> delete[] - more below.

These need to be free calls, sorry about that. The issue is that using malloc
and free fits best with code shared between posix and windows. On posix stuff is
usually allocated with malloc in the system and you free it with free.

https://codereview.chromium.org/11558012/diff/16/runtime/bin/process.cc
File runtime/bin/process.cc (right):

https://codereview.chromium.org/11558012/diff/16/runtime/bin/process.cc#newco...
runtime/bin/process.cc:153: free(os_error_message);
On 2012/12/12 14:00:38, Søren Gjesse wrote:
> Consider allocating os_error_message using new making these all delete[]
calls.
> As far as I can see all these strings are duplicated/allocated in our code.
> 
> In V8 we had the NewArray/DeleteArray to encapsulate this.

For now I'm just consistently using free for all of these. That fits best with
the shared code where we get malloced stuff back from posix systems. I'll be
happy to do a sweep over the code to see if we can switch to using
new[]/delete[] for these as well. It does look weird with the mix here.

Expand Messages | Collapse Messages