Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/Common/src/Interop/Unix/libc/Interop.Environment.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// 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 static partial class Interop
{
internal unsafe partial class libc
{

}
}
24 changes: 24 additions & 0 deletions src/Common/src/Interop/Windows/mincore/Interop.Environment.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// 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 static partial class Interop
{
private static class Libraries
{
internal const string Process = "api-ms-win-core-processenvironment-l1-1-0.dll";
}

internal static unsafe partial class mincore
{
// TODO: Once we have marshalling setup we probably want to revisit these PInvokes
[DllImport(Libraries.Process, EntryPoint = "GetEnvironmentVariableW")]
internal static unsafe extern int GetEnvironmentVariable(char* lpName, char* lpValue, int size);

[DllImport(Libraries.Process, EntryPoint = "ExpandEnvironmentStringsW")]
internal static unsafe extern int ExpandEnvironmentStrings(char* lpSrc, char* lpDst, int nSize);
}
}
10 changes: 9 additions & 1 deletion src/System.Private.CoreLib/src/System.Private.CoreLib.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@
<Compile Include="System\Double.cs" />
<Compile Include="System\Enum.cs" />
<Compile Include="System\Environment.cs" />
<Compile Include="System\Environment.EnvironmentVariables.cs" />
<Compile Include="System\Environment.EnvironmentVariables.cs" />
<Compile Include="System\EventArgs.cs" />
<Compile Include="System\EventHandler.cs" />
<Compile Include="System\FormatException.cs" />
Expand Down Expand Up @@ -477,6 +477,11 @@
<Compile Include="System\Globalization\CultureData.Win32.cs" />
<Compile Include="System\Globalization\TextInfo.Win32.cs" />
<Compile Include="System\Globalization\CalendarData.Win32.cs" />

<Compile Condition="'$(IsProjectNLibrary)' == 'true'" Include="System\Environment.EnvironmentVariables.UWP.cs" />
<!-- For CoreRT we have a different implementation -->
<Compile Condition="'$(IsProjectNLibrary)' != 'true'" Include="System\Environment.EnvironmentVariables.Win32.cs" />
<Compile Include="..\..\Common\src\Interop\Windows\mincore\Interop.Environment.cs" />
</ItemGroup>

<ItemGroup Condition="'$(OSGroup)' == 'Linux'">
Expand All @@ -485,6 +490,9 @@
<Compile Include="System\Globalization\CultureData.Dummy.cs" />
<Compile Include="System\Globalization\TextInfo.Dummy.cs" />
<Compile Include="System\Globalization\CalendarData.Dummy.cs" />
<!-- For CoreRT we have a different implementation -->
<Compile Include="System\Environment.EnvironmentVariables.Unix.cs" />
<Compile Include="..\..\Common\src\Interop\Unix\libc\Interop.Environment.cs" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

/*============================================================
**
**
** Purpose: Provides some basic access to some environment
** functionality.
**
**
============================================================*/

using System.Text;
using System.Collections;

namespace System
{
public static partial class Environment
{
public unsafe static String ExpandEnvironmentVariables(String name)
{
if (name == null)
throw new ArgumentNullException("name");

// Environment variable accessors are not approved modern API.
// Behave as if no variables are defined in this case.
return name;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this is the current implementation exist in PN today. right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

}

public unsafe static String GetEnvironmentVariable(String variable)
{
if (variable == null)
throw new ArgumentNullException("variable");

// Environment variable accessors are not approved modern API.
// Behave as if the variable was not found in this case.
return null;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

/*============================================================
**
**
** Purpose: Provides some basic access to some environment
** functionality.
**
**
============================================================*/

using System.Text;
using System.Collections;

namespace System
{
public static partial class Environment
{
public unsafe static String ExpandEnvironmentVariables(String name)
{
if (name == null)
throw new ArgumentNullException("name");

//TODO: Unix implementations
return name;
}

public unsafe static String GetEnvironmentVariable(String variable)
{
if (variable == null)
throw new ArgumentNullException("variable");

//TODO: Unix implementations
return null;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

/*============================================================
**
**
** Purpose: Provides some basic access to some environment
** functionality.
**
**
============================================================*/

using System.Text;
using System.Collections;

namespace System
{
public static partial class Environment
{
public unsafe static String ExpandEnvironmentVariables(String name)
{
if (name == null)
throw new ArgumentNullException("name");

if (name.Length == 0)
{
return name;
}

int currentSize = 128;
char* blob = stackalloc char[currentSize]; // A somewhat reasonable default size
int requiredSize;
fixed (char* pName = name)
{
requiredSize = Interop.mincore.ExpandEnvironmentStrings(pName, blob, currentSize);
}

if (requiredSize == 0)
{
// TODO: This used to throw an exception:
// Marshal.ThrowExceptionForHR(Marshal.GetHRForLastWin32Error());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yizhang82 do we support Marshal.GetLastWin32Error in PN?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we do - but it is in System.Runtime.InteropServices.dll contract that cannot be used from System.Private.CoreLib.dll.

In System.Private.CoreLib.dll, you have to PInvoke GetLastError() instead.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Check how it is done everywhere else in System.Private.CoreLib.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can get the Last error but we don't have a good way to map that back to an exception -- so I am going to throw ArgumentException for now.

throw new ArgumentException();
}

if (requiredSize <= currentSize)
{
return new string(blob);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add assert here size>currentSize

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If size<=currentSize, it means the first call with the 128 byte buffer succeeded. It doesn't sound like a condition we need to guard against.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are right. the check is not needed

// Fallback to using heap allocated buffers.
char[] newBlob = null;
while (requiredSize > currentSize)
{
currentSize = requiredSize;
newBlob = new char[currentSize];

fixed (char* pName = name, pBlob = newBlob)
{
requiredSize = Interop.mincore.ExpandEnvironmentStrings(pName, pBlob, currentSize);
}

if (requiredSize == 0)
{
// TODO: This used to throw an exception:
// Marshal.ThrowExceptionForHR(Marshal.GetHRForLastWin32Error());
throw new ArgumentException();
}
}

return new string(newBlob);
}

public unsafe static String GetEnvironmentVariable(String variable)
{
if (variable == null)
throw new ArgumentNullException("variable");

// The convention of the API is as follows:
// You call the API with a buffer of a given size.
// If the size of the buffer is insufficient to hold the value,
// the API will return the required size for the buffer.
// In that case we resize the buffer and try again.

int currentSize = 128;
char* blob = stackalloc char[currentSize]; // A somewhat reasonable default size

int requiredSize;
fixed (char* pText = variable)
{
requiredSize = Interop.mincore.GetEnvironmentVariable(pText, blob, currentSize);
}

if (requiredSize == 0)
{
return null;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to throw here too?

}

if (requiredSize <= currentSize)
{
return new string(blob);
}

// Fallback to using heap allocated buffers.
char[] newblob = null;
while (requiredSize > currentSize)
{
currentSize = requiredSize;
// need to retry since the environment variable might be changed
newblob = new char[currentSize];
fixed (char* pText = variable, pBlob = newblob)
{
requiredSize = Interop.mincore.GetEnvironmentVariable(pText, pBlob, currentSize);
}

if (requiredSize == 0)
{
return null;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we do. When getting the value of an envvar we return null in error cases (at least this is what we do on CoreCLR).

}
}
// We should never end up with a null blob
Diagnostics.Debug.Assert(newblob != null);

return new string(newblob);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,30 +15,11 @@

namespace System
{

public static partial class Environment
{
private const int MaxEnvVariableValueLength = 32767; // maximum length for environment variable name and value
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This constant should be also in the platform specific file. I do not think that Unix has any limitation like this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The constant is used by the methods that we are not yet making platform specific (SetEnvironmentVariable, CheckEnvironmentVariableName). I will leave it there until we have platform specifc implementations.


public static String ExpandEnvironmentVariables(String name)
{
if (name == null)
throw new ArgumentNullException("name");

// Environment variable accessors are not approved modern API.
// Behave as if no variables are defined in this case.
return name;
}

public static String GetEnvironmentVariable(String variable)
{
if (variable == null)
throw new ArgumentNullException("variable");

// Environment variable accessors are not approved modern API.
// Behave as if the variable was not found in this case.
return null;
}

public static IDictionary GetEnvironmentVariables()
{
// Environment variable accessors are not approved modern API.
Expand Down