Implement GetEnvironmentVariable and ExpandEnvironmentStrings on Windows #147
Conversation
| internal partial class Interop | ||
| { | ||
| internal unsafe partial class Environment | ||
| { |
There was a problem hiding this comment.
Take a look how the interop definitions are structured in corefx. I think we may want to do the same thing here.
There was a problem hiding this comment.
Changed this to Interop.Sys. ...
There was a problem hiding this comment.
According to https://github.com/dotnet/corefx/blob/master/Documentation/coding-guidelines/interop-guidelines.md, it should be class Interop { class mincore, the file should live under Common, and the filename should be something like Interop.Environment.cs
There was a problem hiding this comment.
I was unaware we had interop guidelines spelled out!
Thanks for the link!
Will update according to them.
Alex
From: Jan Kotas [mailto:notifications@github.com]
Sent: Thursday, October 29, 2015 12:25 PM
To: dotnet/corert corert@noreply.github.com
Cc: Alexandru Ghiondea Ghiondea.Alexandru@microsoft.com
Subject: Re: [corert] Implement GetEnvironmentVariable and ExpandEnvironmentStrings on Windows (#147)
In src/System.Private.CoreLib/src/Interop/Interop.Win32.cshttps://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2fdotnet%2fcorert%2fpull%2f147%23discussion_r43432571&data=01%7c01%7cghiondea.alexandru%40microsoft.com%7c3d3295ebf3e64e44e3ae08d2e096a154%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=t1LGWRhWVYY87yotQU6hJM3MspR2Ra6tEowJqP0PIeM%3d:
@@ -0,0 +1,19 @@
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+using System;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+internal partial class Interop
+{
- internal unsafe partial class Environment
- {
According to https://github.com/dotnet/corefx/blob/master/Documentation/coding-guidelines/interop-guidelines.mdhttps://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2fdotnet%2fcorefx%2fblob%2fmaster%2fDocumentation%2fcoding-guidelines%2finterop-guidelines.md&data=01%7c01%7cghiondea.alexandru%40microsoft.com%7c3d3295ebf3e64e44e3ae08d2e096a154%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=Zyi9xOWsvYXSCIC9ISm4bF%2f32GS54ntVo4dURmszkRc%3d, it should be class Interop { class mincore, the file should live under Common, and the filename should be something like Interop.Environment.cs
—
Reply to this email directly or view it on GitHubhttps://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2fdotnet%2fcorert%2fpull%2f147%2ffiles%23r43432571&data=01%7c01%7cghiondea.alexandru%40microsoft.com%7c3d3295ebf3e64e44e3ae08d2e096a154%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=9PMKP3BHP4B%2b802jcdCXeglcmJBglMNic9IVgXOXtXs%3d.
|
Will rebase and squash before I merge this in. |
|
Added the missing commit. |
|
|
||
| if (requiredSize == 0) | ||
| { | ||
| return null; |
|
add some little comment but other than that LGTM |
…ows. The change refactors the Environment.EnvironmentVariables into 3 parts: - Environment.EnvironmentVariables.UWP.cs -> This contains the implementations that we want to carry over for UWP apps - Environment.EnvironmentVariables.Win32.cs -> This contains the implementation we want to have for CoreRT on Windows - Environment.EnvironmentVariables.Unix.cs -> This contains the implementation we want to have for CoreRT on Unix I have also introduced 2 Interop files (Win32 and Unix) that will contain the corresponding PInvokes.
Implement GetEnvironmentVariable and ExpandEnvironmentStrings on Windows
The change refactors the Environment.EnvironmentVariables into 3 parts:
-> This contains the implementations that we want to carry over for UWP apps
-> This contains the implementation we want to have for CoreRT on Windows
-> This contains the implementation we want to have for CoreRT on Unix
I have also introduced 2 Interop files (Win32 and Unix) that will contain the
corresponding PInvokes.
Partially fixes #6