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

Issue 10112002: Add read-only environment variable access. (Closed)

Created:
8 years, 8 months ago by Mads Ager (google)
Modified:
8 years, 8 months ago
Reviewers:
Søren Gjesse
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Add read-only environment variable access. Map<String, String> env = new Platform().environment(); Unfortunately, this is a bit hard to test at this point. I will move on to implement support for providing an environment to processes that are started. At that point we can test it properly. Another follow-up change will be to remove the instantiation for Platform. I would like this to be just Map<String, String> env = Platform.environment(); R=sgjesse@google.com BUG=dartbug.com/752 TEST= Committed: https://code.google.com/p/dart/source/detail?r=6646

Patch Set 1 #

Patch Set 2 : Add windows error handling. #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+125 lines, -11 lines) Patch
M runtime/bin/builtin_natives.cc View 1 chunk +1 line, -0 lines 0 comments Download
M runtime/bin/platform.h View 2 chunks +8 lines, -2 lines 0 comments Download
M runtime/bin/platform.cc View 1 2 chunks +32 lines, -1 line 0 comments Download
M runtime/bin/platform.dart View 1 chunk +5 lines, -0 lines 0 comments Download
M runtime/bin/platform_impl.dart View 3 chunks +17 lines, -1 line 0 comments Download
M runtime/bin/platform_linux.cc View 2 chunks +16 lines, -1 line 2 comments Download
M runtime/bin/platform_macos.cc View 2 chunks +20 lines, -1 line 0 comments Download
M runtime/bin/platform_win.cc View 1 2 chunks +23 lines, -4 lines 0 comments Download
M tests/standalone/src/io/PlatformTest.dart View 2 chunks +3 lines, -1 line 0 comments Download

Messages

Total messages: 3 (0 generated)
Mads Ager (google)
8 years, 8 months ago (2012-04-17 15:16:14 UTC) #1
Søren Gjesse
lgtm Not that bad after all. http://codereview.chromium.org/10112002/diff/3001/runtime/bin/platform_linux.cc File runtime/bin/platform_linux.cc (right): http://codereview.chromium.org/10112002/diff/3001/runtime/bin/platform_linux.cc#newcode45 runtime/bin/platform_linux.cc:45: intptr_t i = ...
8 years, 8 months ago (2012-04-17 16:10:47 UTC) #2
Mads Ager (google)
8 years, 8 months ago (2012-04-17 16:20:14 UTC) #3
http://codereview.chromium.org/10112002/diff/3001/runtime/bin/platform_linux.cc
File runtime/bin/platform_linux.cc (right):

http://codereview.chromium.org/10112002/diff/3001/runtime/bin/platform_linux....
runtime/bin/platform_linux.cc:45: intptr_t i = 0;
On 2012/04/17 16:10:47, Søren Gjesse wrote:
> Why add i? Maybe just use *count instead.

I wanted to have on clear assignment to count. Also, I found it easier to read
because of the occurrences of 'i' below as well.

Powered by Google App Engine
This is Rietveld 408576698