Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade the .NET bindings to .NET Standard 2.0. #410

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion CREDITS.TXT
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,5 @@ Ruben Boonen: PowerShell binding.
Marco Fornaro: C# binding.
David Zimmer: VB6 binding.
Michael Mohr: Debian packaging.
Jämes Ménétrey (ZenLulz): Java binding.
Jämes Ménétrey (ZenLulz): Java and C# bindings.
Philippe Antoine (Catena cyber): fuzzing.
2 changes: 1 addition & 1 deletion bindings/README
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Except Python, all other bindings are contributed by community.
- Haskell binding: by Adrian Herrera
- OCaml binding: by Aziem Chawdhary
- PowerShell binding: by Ruben Boonen
- C# binding: by Marco Fornaro
- C# binding: by Marco Fornaro and Jämes Ménétrey (ZenLulz)
- VB6 binding: by David Zimmer
- Masm binding: by mrfearless
- Java binding: by Jämes Ménétrey (ZenLulz)
Expand Down
2 changes: 0 additions & 2 deletions bindings/csharp/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,6 @@ publish/
#*.pubxml
*.publishproj

# NuGet Packages
*.nupkg
# The packages folder can be ignored because of Package Restore
**/packages/*
# except build/, which is used as an MSBuild target.
Expand Down
11 changes: 11 additions & 0 deletions bindings/csharp/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Changelog of the .NET bindings for Keystone

**Version 0.9.1.1: May 18th, 2019**:

- Update to .NET Standard 2.0.
- Load dynamically the dynamic-link library that corresponds to the architecture of the running process (32 or 64-bit).


**Version 0.9.1.0: August 2nd, 2018**:

- Refactor the bindings to target .NET Standard 1.1.
Empty file modified bindings/csharp/Keystone.Net.Tests/Keystone.Net.Tests.csproj
100644 → 100755
Empty file.
34 changes: 21 additions & 13 deletions bindings/csharp/Keystone.Net/Keystone.Net.csproj
100644 → 100755
Original file line number Diff line number Diff line change
@@ -1,28 +1,36 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>netstandard1.1</TargetFramework>
<TargetFramework>netstandard2.0</TargetFramework>
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're not using features only available in .NET Standard 2.0 (AFAIK), I think we should try targeting multiple frameworks by replacing

<TargetFramework>netstandard2.0</TargetFramework>

by

<TargetFrameworks>netstandard1.1;netstandard2.0</TargetFrameworks>

<RootNamespace>Keystone</RootNamespace>

<Version>1.1.0</Version>
<AssemblyVersion>$(Version)</AssemblyVersion>
<FileVersion>$(Version).0</FileVersion>
<Version>0.9.1.1</Version>
<AssemblyVersion>0.9.1.1</AssemblyVersion>
<FileVersion>0.9.1.1</FileVersion>

<Description>.NET bindings to the Keystone Engine.</Description>
<Authors>Grégoire Geis</Authors>
<Description>Keystone is a lightweight multi-platform, multi-architecture assembler framework. This package corresponds to the csharp bindings in the official git repository.</Description>
<Authors>Nguyen Anh Quynh, Marco Fornaro, Grégoire Geis, Jämes Ménétrey</Authors>

<PackageId>Keystone.Net</PackageId>
<PackageId>keystoneengine.csharp</PackageId>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why renaming to KeystoneEngine.CSharp? Usually packages that end with .CSharp are related to the language itself (ie. Microsoft.CodeAnalysis.CSharp). Bindings are available to all of .NET (not only C#) and thus usually end with .NET.

However this does pose a problem: How do we decrease the current version? (I'm not sure NuGet allows this).

<PackageVersion>$(Version)</PackageVersion>
<PackageRequireLicenseAcceptance>False</PackageRequireLicenseAcceptance>
<PackageReleaseNotes>- First release.</PackageReleaseNotes>
<PackageTags>assembler x86 arm keystone</PackageTags>
<PackageRequireLicenseAcceptance>true</PackageRequireLicenseAcceptance>
<PackageReleaseNotes>Release notes can be found at: https://github.com/keystone-engine/keystone/blob/0.9.1/ChangeLog</PackageReleaseNotes>
<PackageTags>assembler x86 x64 arm keystone llvm</PackageTags>

<PackageProjectUrl>https://github.com/keystone-engine/keystone</PackageProjectUrl>
<PackageLicenseUrl>$(PackageProjectUrl)/blob/master/COPYING</PackageLicenseUrl>
<PackageProjectUrl>http://www.keystone-engine.org</PackageProjectUrl>
<PackageLicenseUrl>https://github.com/keystone-engine/keystone#license</PackageLicenseUrl>
<PackageIconUrl>http://www.keystone-engine.org/images/keystone.png</PackageIconUrl>

<RepositoryUrl>$(PackageProjectUrl).git</RepositoryUrl>
<RepositoryUrl>https://github.com/keystone-engine/keystone</RepositoryUrl>
<RepositoryType>git</RepositoryType>
<Company />
<Copyright>Nguyen Anh Quynh</Copyright>
<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
<Product>Keystone Engine - .NET Bindings</Product>
</PropertyGroup>

<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|AnyCPU'">
<GenerateDocumentationFile>true</GenerateDocumentationFile>
</PropertyGroup>

</Project>
44 changes: 25 additions & 19 deletions bindings/csharp/Keystone.Net/NativeInterop.cs
100644 → 100755
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.IO;
using System.Runtime.InteropServices;

namespace Keystone
Expand All @@ -8,29 +9,34 @@ namespace Keystone
/// </summary>
internal class NativeInterop
{
// This shouldn't be needed, even on Windows
// /// <summary>
// /// Taken from: http://stackoverflow.com/questions/10852634/using-a-32bit-or-64bit-dll-in-c-sharp-dllimport
// /// </summary>
// static NativeInterop()
// {
// var myPath = new Uri(typeof(NativeInterop).Assembly.CodeBase).LocalPath;
// var myFolder = Path.GetDirectoryName(myPath);
/// <summary>
/// Load the appropriate dynamic-link library, according the architecture of the running application.
/// </summary>
/// <remarks>
/// Taken from: http://stackoverflow.com/questions/10852634/using-a-32bit-or-64bit-dll-in-c-sharp-dllimport
/// </remarks>
static NativeInterop()
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm kinda curious about this: Is it ever needed? I removed it from the original commit because after tests in different environments I never ended up needing this.

{
var libPath = Path.GetDirectoryName(new Uri(typeof(NativeInterop).Assembly.CodeBase).LocalPath);
var is64 = IntPtr.Size == 8;
var subfolder = is64 ? "x64" : "x86";

// var is64 = IntPtr.Size == 8;
// var subfolder = is64 ? "\\win64\\" : "\\win32\\";
if (!string.IsNullOrEmpty(libPath))
{
var dllPosition = Path.Combine(libPath, subfolder, "keystone.dll");

// string dllPosition = myFolder + subfolder + "keystone.dll";
// If this file exist, load it.
// Otherwise let the marshaller load the appropriate file.
if (File.Exists(dllPosition))
{
LoadLibrary(dllPosition);
}
}
}

// // If this file exist, load it.
// // Otherwise let the marshaller load the appropriate file.
// if (File.Exists(dllPosition))
// LoadLibrary(dllPosition);
// }
[DllImport("kernel32.dll")]
private static extern IntPtr LoadLibrary(string dllToLoad);

// [DllImport("kernel32.dll")]
// private static extern IntPtr LoadLibrary(string dllToLoad);

[DllImport("keystone", CallingConvention = CallingConvention.Cdecl, EntryPoint = "ks_version" )]
internal static extern uint Version(ref uint major, ref uint minor);

Expand Down
37 changes: 35 additions & 2 deletions bindings/csharp/README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Keystone.Net
.NET Standard bindings for Keystone.
# Keystone.Net
.NET bindings for Keystone (.NET Standard 2.0), written in C#.

## Usage
```csharp
Expand Down Expand Up @@ -30,3 +30,36 @@ using (Engine keystone = new Engine(Architecture.X86, Mode.X32) { ThrowOnError =

For those who already used the bindings before their last update, many things have changed.
You can migrate your existing code easily using the [migration guide](./MIGRATON.md).

## NuGet package
The NuGet package `keystoneengine.csharp` is maintained to reflect the latest version of the bindings and the library. It can either be downloaded using Visual Studio or by [browsing NuGet directly](https://www.nuget.org/packages/keystoneengine.csharp/). The package already embeds the 32/64-bit native dynamic-link libraries of Keystone.

## Found an issue or bug ?
Feel free to open a GitHub issue on [the official repository of Keystone](https://github.com/keystone-engine/keystone/issues) and ping the contributors.


## Contributors
Authors:

- Grégoire Geis ([https://github.com/71](@71))
Copy link
Contributor

Choose a reason for hiding this comment

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

Link and text are swapped.

- Jämes Ménétrey ([@ZenLulz](https://github.com/ZenLulz/))
- Marco Fornaro ([@chaplin89](https://github.com/chaplin89))

### Want to contribute ?
Hey you! Your help is more than welcome! Things to keep in mind when working on the .NET bindings for Keystone:

- Think about the backward compatibility; while code refactoring is a good practice, changing entirely the API *may* result in struggles.
- Elaborate the unit tests that prove your code is working. Test all the paths of the newly added functions/classes. Keep the code coverage high!
- Please; write the required *XML Documentation Comments*, so every developer has the chance to understand your code.
- Update the changelog with a summary of your changes.

#### Version notation
The version of the .NET bindings for Keystone is indicated in the Visual Studio project file. The major, minor and incremental versions (w.x.y) match the version of the library Keystone that the bindings are developed with. The build number (the .z in w.x.y.z) is incremented for each newer version of the .NET bindings. Please, don't forget to increment this version when you submit a pull request.

On the last commit for a pull request, please create a tag called `csharp-bindings-w.x.y.z`.

#### Pull request submission
Ping the contributors of the .NET bindings when submitting a pull request, so your changes can be peer reviewed.

#### NuGet package update
Once your pull request has been accepted, please contact [@ZenLulz](https://github.com/ZenLulz/) so either he updates the library for you or add you to the project as a contributor on nuget.org. An example of *nupkg* is provided in this folder, as it requires to have a very specific configuration, because it embeds unmanaged libraries. The picture *nuget-package-config.png* details the structure and content of a NuGet package, ready to be deployed. Please reuse and test the package before pushing it onto nuget.org, as there is no possible roll back.
Copy link
Contributor

Choose a reason for hiding this comment

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

If we end up keeping the existing package name, let's remember to modify this line.

Binary file added bindings/csharp/keystoneengine.csharp.nupkg
Binary file not shown.
Binary file added bindings/csharp/nuget-package-config.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.