Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Commit

Permalink
Address Code review Feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@swaroop-sridhar
swaroop-sridhar committed Jan 5, 2019
1 parent f379400 commit b20b30e
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 61 deletions.
63 changes: 35 additions & 28 deletions src/System.Private.CoreLib/src/System/Runtime/InteropServices/NativeLibrary.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ public static partial class NativeLibrary
/// <exception cref="System.ArgumentNullException">If libraryPath is null</exception>
/// <exception cref="System.DllNotFoundException ">If the library can't be found.</exception>
/// <exception cref="System.BadImageFormatException">If the library is not valid.</exception>
public static IntPtr LoadLibrary(string libraryPath)
public static IntPtr Load(string libraryPath)
{
if (libraryPath == null)
throw new ArgumentNullException(nameof(libraryPath));

return LoadLibraryFromPath(libraryPath, throwOnError: true);
return LoadFromPath(libraryPath, throwOnError: true);
}

/// <summary>
Expand All @@ -42,12 +42,12 @@ public static IntPtr LoadLibrary(string libraryPath)
/// <param name="handle">The out-parameter for the loaded native library handle</param>
/// <returns>True on successful load, false otherwise</returns>
/// <exception cref="System.ArgumentNullException">If libraryPath is null</exception>
public static bool TryLoadLibrary(string libraryPath, out IntPtr handle)
public static bool TryLoad(string libraryPath, out IntPtr handle)
{
if (libraryPath == null)
throw new ArgumentNullException(nameof(libraryPath));

handle = LoadLibraryFromPath(libraryPath, throwOnError: false);
handle = LoadFromPath(libraryPath, throwOnError: false);
return handle != IntPtr.Zero;
}

Expand All @@ -69,7 +69,7 @@ public static bool TryLoadLibrary(string libraryPath, out IntPtr handle)
/// <exception cref="System.ArgumentException">If assembly is not a RuntimeAssembly</exception>
/// <exception cref="System.DllNotFoundException ">If the library can't be found.</exception>
/// <exception cref="System.BadImageFormatException">If the library is not valid.</exception>
public static IntPtr LoadLibrary(string libraryName, Assembly assembly, DllImportSearchPath? searchPath)
public static IntPtr Load(string libraryName, Assembly assembly, DllImportSearchPath? searchPath)
{
if (libraryName == null)
throw new ArgumentNullException(nameof(libraryName));
Expand All @@ -78,11 +78,11 @@ public static IntPtr LoadLibrary(string libraryName, Assembly assembly, DllImpor
if (!(assembly is RuntimeAssembly))
throw new ArgumentException(SR.Argument_MustBeRuntimeAssembly);

return LoadLibraryByName(libraryName,
((RuntimeAssembly)assembly).GetNativeHandle(),
searchPath.HasValue,
(uint) searchPath.GetValueOrDefault(),
throwOnError: true);
return LoadByName(libraryName,
((RuntimeAssembly)assembly).GetNativeHandle(),
searchPath.HasValue,
(uint) searchPath.GetValueOrDefault(),
throwOnError: true);
}

/// <summary>
Expand All @@ -95,7 +95,7 @@ public static IntPtr LoadLibrary(string libraryName, Assembly assembly, DllImpor
/// <returns>True on successful load, false otherwise</returns>
/// <exception cref="System.ArgumentNullException">If libraryPath or assembly is null</exception>
/// <exception cref="System.ArgumentException">If assembly is not a RuntimeAssembly</exception>
public static bool TryLoadLibrary(string libraryName, Assembly assembly, DllImportSearchPath? searchPath, out IntPtr handle)
public static bool TryLoad(string libraryName, Assembly assembly, DllImportSearchPath? searchPath, out IntPtr handle)
{
if (libraryName == null)
throw new ArgumentNullException(nameof(libraryName));
Expand All @@ -104,11 +104,11 @@ public static bool TryLoadLibrary(string libraryName, Assembly assembly, DllImpo
if (!(assembly is RuntimeAssembly))
throw new ArgumentException(SR.Argument_MustBeRuntimeAssembly);

handle = LoadLibraryByName(libraryName,
((RuntimeAssembly)assembly).GetNativeHandle(),
searchPath.HasValue,
(uint) searchPath.GetValueOrDefault(),
throwOnError: false);
handle = LoadByName(libraryName,
((RuntimeAssembly)assembly).GetNativeHandle(),
searchPath.HasValue,
(uint) searchPath.GetValueOrDefault(),
throwOnError: false);
return handle != IntPtr.Zero;
}

Expand All @@ -119,9 +119,9 @@ public static bool TryLoadLibrary(string libraryName, Assembly assembly, DllImpo
/// </summary>
/// <param name="handle">The native library handle to be freed</param>
/// <exception cref="System.InvalidOperationException">If the operation fails</exception>
public static void FreeLibrary(IntPtr handle)
public static void Free(IntPtr handle)
{
FreeNativeLibrary(handle);
FreeLib(handle);
}

/// <summary>
Expand All @@ -133,14 +133,14 @@ public static void FreeLibrary(IntPtr handle)
/// <returns>The address of the symbol</returns>
/// <exception cref="System.ArgumentNullException">If handle or name is null</exception>
/// <exception cref="System.EntryPointNotFoundException">If the symbol is not found</exception>
public static IntPtr GetLibraryExport(IntPtr handle, string name)
public static IntPtr GetExport(IntPtr handle, string name)
{
if (handle == IntPtr.Zero)
throw new ArgumentNullException(nameof(handle));
if (name == null)
throw new ArgumentNullException(nameof(name));

return GetNativeLibraryExport(handle, name, throwOnError: true);
return GetSymbol(handle, name, throwOnError: true);
}

/// <summary>
Expand All @@ -151,28 +151,35 @@ public static IntPtr GetLibraryExport(IntPtr handle, string name)
/// <param name="address"> The out-parameter for the symbol address, if it exists</param>
/// <returns>True on success, false otherwise</returns>
/// <exception cref="System.ArgumentNullException">If handle or name is null</exception>
public static bool TryGetLibraryExport(IntPtr handle, string name, out IntPtr address)
public static bool TryGetExport(IntPtr handle, string name, out IntPtr address)
{
if (handle == IntPtr.Zero)
throw new ArgumentNullException(nameof(handle));
if (name == null)
throw new ArgumentNullException(nameof(name));

address = GetNativeLibraryExport(handle, name, throwOnError: false);
address = GetSymbol(handle, name, throwOnError: false);
return address != IntPtr.Zero;
}

/// External functions that implement the NativeLibrary interface

[DllImport(JitHelpers.QCall, CharSet = CharSet.Unicode)]
internal static extern IntPtr LoadLibraryFromPath(string libraryName, bool throwOnError);
internal static extern IntPtr LoadFromPath(string libraryName,
[MarshalAs(UnmanagedType.Bool)] bool throwOnError);

[DllImport(JitHelpers.QCall, CharSet = CharSet.Unicode)]
internal static extern IntPtr LoadLibraryByName(string libraryName, RuntimeAssembly callingAssembly,
bool hasDllImportSearchPathFlag, uint dllImportSearchPathFlag,
bool throwOnError);
internal static extern IntPtr LoadByName(string libraryName, RuntimeAssembly callingAssembly,
[MarshalAs(UnmanagedType.Bool)] bool hasDllImportSearchPathFlag,
uint dllImportSearchPathFlag,
[MarshalAs(UnmanagedType.Bool)] bool throwOnError);

[DllImport(JitHelpers.QCall, CharSet = CharSet.Unicode)]
internal static extern void FreeNativeLibrary(IntPtr handle);
internal static extern void FreeLib(IntPtr handle);

[DllImport(JitHelpers.QCall, CharSet = CharSet.Unicode)]
internal static extern IntPtr GetNativeLibraryExport(IntPtr handle, string symbolName, bool throwOnError);
internal static extern IntPtr GetSymbol(IntPtr handle,
string symbolName,
[MarshalAs(UnmanagedType.Bool)] bool throwOnError);
}
}
8 changes: 4 additions & 4 deletions src/vm/ecalllist.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -876,10 +876,10 @@ FCFuncStart(gInteropMarshalFuncs)
FCFuncEnd()

FCFuncStart(gInteropNativeLibraryFuncs)
QCFuncElement("LoadLibraryFromPath", NativeLibraryNative::LoadLibraryFromPath)
QCFuncElement("LoadLibraryByName", NativeLibraryNative::LoadLibraryByName)
QCFuncElement("FreeNativeLibrary", NativeLibraryNative::FreeNativeLibrary)
QCFuncElement("GetNativeLibraryExport", NativeLibraryNative::GetNativeLibraryExport)
QCFuncElement("LoadFromPath", NativeLibraryNative::LoadFromPath)
QCFuncElement("LoadByName", NativeLibraryNative::LoadByName)
QCFuncElement("FreeLib", NativeLibraryNative::FreeLib)
QCFuncElement("GetSymbol", NativeLibraryNative::GetSymbol)
FCFuncEnd()

FCFuncStart(gArrayWithOffsetFuncs)
Expand Down
12 changes: 4 additions & 8 deletions src/vm/nativelibrarynative.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,12 @@
// File: NativeLibraryNative.cpp
//

//
// FCall's for the PInvoke classlibs
//

#include "common.h"
#include "dllimport.h"
#include "nativelibrarynative.h"

// static
INT_PTR QCALLTYPE NativeLibraryNative::LoadLibraryFromPath(LPCWSTR path, BOOL throwOnError)
INT_PTR QCALLTYPE NativeLibraryNative::LoadFromPath(LPCWSTR path, BOOL throwOnError)
{
QCALL_CONTRACT;

Expand All @@ -30,7 +26,7 @@ INT_PTR QCALLTYPE NativeLibraryNative::LoadLibraryFromPath(LPCWSTR path, BOOL th
}

// static
INT_PTR QCALLTYPE NativeLibraryNative::LoadLibraryByName(LPCWSTR name, QCall::AssemblyHandle callingAssembly,
INT_PTR QCALLTYPE NativeLibraryNative::LoadByName(LPCWSTR name, QCall::AssemblyHandle callingAssembly,
BOOL hasDllImportSearchPathFlag, DWORD dllImportSearchPathFlag,
BOOL throwOnError)
{
Expand All @@ -49,7 +45,7 @@ INT_PTR QCALLTYPE NativeLibraryNative::LoadLibraryByName(LPCWSTR name, QCall::As
}

// static
void QCALLTYPE NativeLibraryNative::FreeNativeLibrary(INT_PTR handle)
void QCALLTYPE NativeLibraryNative::FreeLib(INT_PTR handle)
{
QCALL_CONTRACT;

Expand All @@ -61,7 +57,7 @@ void QCALLTYPE NativeLibraryNative::FreeNativeLibrary(INT_PTR handle)
}

//static
INT_PTR QCALLTYPE NativeLibraryNative::GetNativeLibraryExport(INT_PTR handle, LPCWSTR symbolName, BOOL throwOnError)
INT_PTR QCALLTYPE NativeLibraryNative::GetSymbol(INT_PTR handle, LPCWSTR symbolName, BOOL throwOnError)
{
QCALL_CONTRACT;

Expand Down
11 changes: 4 additions & 7 deletions src/vm/nativelibrarynative.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,22 @@
//
// File: NativeLibraryNative.h
//

//
// QCall's for the NativeLibrary class
//


#ifndef __NATIVELIBRARYNATIVE_H__
#define __NATIVELIBRARYNATIVE_H__


class NativeLibraryNative
{
public:
static INT_PTR QCALLTYPE LoadLibraryFromPath(LPCWSTR path, BOOL throwOnError);
static INT_PTR QCALLTYPE LoadLibraryByName(LPCWSTR name, QCall::AssemblyHandle callingAssembly,
static INT_PTR QCALLTYPE LoadFromPath(LPCWSTR path, BOOL throwOnError);
static INT_PTR QCALLTYPE LoadByName(LPCWSTR name, QCall::AssemblyHandle callingAssembly,
BOOL hasDllImportSearchPathFlag, DWORD dllImportSearchPathFlag,
BOOL throwOnError);
static void QCALLTYPE FreeNativeLibrary(INT_PTR handle);
static INT_PTR QCALLTYPE GetNativeLibraryExport(INT_PTR handle, LPCWSTR symbolName, BOOL throwOnError);
static void QCALLTYPE FreeLib(INT_PTR handle);
static INT_PTR QCALLTYPE GetSymbol(INT_PTR handle, LPCWSTR symbolName, BOOL throwOnError);

};

Expand Down
28 changes: 14 additions & 14 deletions tests/src/Interop/NativeLibrary/NativeLibraryTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ public static int Main()
// -----------------------------------------------

libName = Path.Combine(testBinDir, GetNativeLibraryName());
handle = NativeLibrary.LoadLibrary(libName);
handle = NativeLibrary.Load(libName);

// Valid Free
success &= EXPECT(FreeLibrary(handle));
Expand All @@ -153,7 +153,7 @@ public static int Main()
// GetLibraryExport Tests
// -----------------------------------------------
libName = Path.Combine(testBinDir, GetNativeLibraryName());
handle = NativeLibrary.LoadLibrary(libName);
handle = NativeLibrary.Load(libName);

// Valid Call (with some hard-coded name mangling)
success &= EXPECT(GetLibraryExport(handle, TestLibrary.Utilities.IsX86 ? "_NativeSum@8" : "NativeSum"));
Expand All @@ -171,7 +171,7 @@ public static int Main()
success &= EXPECT(GetLibraryExport(handle, "NonNativeSum"), TestResult.EntryPointNotFound);
success &= EXPECT(TryGetLibraryExport(handle, "NonNativeSum"), TestResult.ReturnFailure);

NativeLibrary.FreeLibrary(handle);
NativeLibrary.Free(handle);
}
catch (Exception e)
{
Expand Down Expand Up @@ -279,13 +279,13 @@ static TestResult LoadLibrarySimple(string libPath)
IntPtr handle = IntPtr.Zero;

TestResult result = Run(() => {
handle = NativeLibrary.LoadLibrary(libPath);
handle = NativeLibrary.Load(libPath);
if (handle == IntPtr.Zero)
return TestResult.ReturnNull;
return TestResult.Success;
});

NativeLibrary.FreeLibrary(handle);
NativeLibrary.Free(handle);

return result;
}
Expand All @@ -297,15 +297,15 @@ static TestResult TryLoadLibrarySimple(string libPath)
IntPtr handle = IntPtr.Zero;

TestResult result = Run(() => {
bool success = NativeLibrary.TryLoadLibrary(libPath, out handle);
bool success = NativeLibrary.TryLoad(libPath, out handle);
if(!success)
return TestResult.ReturnFailure;
if (handle == null)
return TestResult.ReturnNull;
return TestResult.Success;
});

NativeLibrary.FreeLibrary(handle);
NativeLibrary.Free(handle);

return result;
}
Expand All @@ -318,13 +318,13 @@ static TestResult LoadLibraryAdvanced(string libName, Assembly assembly, DllImpo
IntPtr handle = IntPtr.Zero;

TestResult result = Run(() => {
handle = NativeLibrary.LoadLibrary(libName, assembly, searchPath);
handle = NativeLibrary.Load(libName, assembly, searchPath);
if (handle == IntPtr.Zero)
return TestResult.ReturnNull;
return TestResult.Success;
});

NativeLibrary.FreeLibrary(handle);
NativeLibrary.Free(handle);

return result;
}
Expand All @@ -336,15 +336,15 @@ static TestResult TryLoadLibraryAdvanced(string libName, Assembly assembly, DllI
IntPtr handle = IntPtr.Zero;

TestResult result = Run(() => {
bool success = NativeLibrary.TryLoadLibrary(libName, assembly, searchPath, out handle);
bool success = NativeLibrary.TryLoad(libName, assembly, searchPath, out handle);
if (!success)
return TestResult.ReturnFailure;
if (handle == IntPtr.Zero)
return TestResult.ReturnNull;
return TestResult.Success;
});

NativeLibrary.FreeLibrary(handle);
NativeLibrary.Free(handle);

return result;
}
Expand All @@ -354,7 +354,7 @@ static TestResult FreeLibrary(IntPtr handle)
CurrentTest = String.Format("FreeLibrary({0})", handle);

return Run(() => {
NativeLibrary.FreeLibrary(handle);
NativeLibrary.Free(handle);
return TestResult.Success;
});
}
Expand All @@ -364,7 +364,7 @@ static TestResult GetLibraryExport(IntPtr handle, string name)
CurrentTest = String.Format("GetLibraryExport({0}, {1})", handle, name);

return Run(() => {
IntPtr address = NativeLibrary.GetLibraryExport(handle, name);
IntPtr address = NativeLibrary.GetExport(handle, name);
if (address == null)
return TestResult.ReturnNull;
if (RunExportedFunction(address, 1, 1) != 2)
Expand All @@ -379,7 +379,7 @@ static TestResult TryGetLibraryExport(IntPtr handle, string name)

return Run(() => {
IntPtr address = IntPtr.Zero;
bool success = NativeLibrary.TryGetLibraryExport(handle, name, out address);
bool success = NativeLibrary.TryGetExport(handle, name, out address);
if (!success)
return TestResult.ReturnFailure;
if (address == null)
Expand Down

0 comments on commit b20b30e

Please sign in to comment.