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.

Sign up for GitHub

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

Fix undefined layout behaviour, and remove pinning: Uint256 and Uint160 #1387

Merged
merged 17 commits into from
Jan 15, 2020
Merged

Fix undefined layout behaviour, and remove pinning: Uint256 and Uint160 #1387

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
d204cc8
Fix undefined layout behaviour, and remove pinning
john-h-k Dec 30, 2019
799b6e9
Update UInt256.cs
john-h-k Dec 30, 2019
cde8bb4
Update UInt256.cs
john-h-k Dec 30, 2019
b073565
add StructLayout
Jan 10, 2020
e53a2be
set pack=1
Jan 10, 2020
c856c07
Merge branch 'master' into patch-1
vncoelho Jan 10, 2020
b9fd397
Merging Tommo branch
vncoelho Jan 10, 2020
6b52a79
Explicit
erikzhang Jan 12, 2020
bcdf640
Remove UIntBase
erikzhang Jan 12, 2020
8b6528b
Revert constructor
shargon Jan 13, 2020
cf655fa
add unsafe
shargon Jan 13, 2020
57c5bb2
Merge branch 'master' into patch-1
vncoelho Jan 13, 2020
5e553bf
Comment
vncoelho Jan 13, 2020
7054db9
Comment Uint256
vncoelho Jan 13, 2020
8a9fdf7
Removing comments from value 1
vncoelho Jan 14, 2020
a472cfe
Removing comments from value1
vncoelho Jan 14, 2020
4624e60
Merge branch 'master' into patch-1
vncoelho Jan 14, 2020
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: 2 additions & 0 deletions src/neo/UInt160.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
using System;
using System.Globalization;
using System.IO;
using System.Runtime.InteropServices;

namespace Neo
{
/// <summary>
/// This class stores a 160 bit unsigned int, represented as a 20-byte little-endian byte array
/// </summary>
[StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi, Pack = 1)]
public class UInt160 : UIntBase, IComparable<UInt160>, IEquatable<UInt160>
{
public const int Length = 20;
Expand Down
11 changes: 5 additions & 6 deletions src/neo/UInt256.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
using System;
using System.Globalization;
using System.IO;
using System.Runtime.InteropServices;

namespace Neo
{
/// <summary>
/// This class stores a 256 bit unsigned int, represented as a 32-byte little-endian byte array
/// </summary>
[StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi, Pack = 1)]
public class UInt256 : UIntBase, IComparable<UInt256>, IEquatable<UInt256>
{
public const int Length = 32;
Expand All @@ -23,13 +25,10 @@ public UInt256()
{
}

public unsafe UInt256(ReadOnlySpan<byte> value)
public UInt256(ReadOnlySpan<byte> value)
{
fixed (ulong* p = &value1)
{
Span<byte> dst = new Span<byte>(p, Length);
value[..Length].CopyTo(dst);
}
Span<byte> dst = MemoryMarshal.AsBytes(MemoryMarshal.CreateSpan(ref value1, Length / sizeof(ulong)));
value[..Length].CopyTo(dst);
}
shargon marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
Expand Down
2 changes: 2 additions & 0 deletions src/neo/UIntBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
using Neo.IO;
using System;
using System.IO;
using System.Runtime.InteropServices;

namespace Neo
{
/// <summary>
/// Base class for little-endian unsigned integers. Two classes inherit from this: UInt160 and UInt256.
/// Only basic comparison/serialization are proposed for these classes. For arithmetic purposes, use BigInteger class.
/// </summary>
[StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi, Pack = 1)]
Copy link
Member

Choose a reason for hiding this comment

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

This is not needed and should be removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

We need this line, or replace by [StructLayout(LayoutKind.Explicit, Size = 0)], as UIntBase is the base class which also needs this, otherwise the ut can't pass.

Copy link
Member

Choose a reason for hiding this comment

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

I removed UIntBase.

Copy link
Member

Choose a reason for hiding this comment

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

We can remove it if you want, it's not used

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure if we need it, maybe the compiler optimizations will need it

public abstract class UIntBase : ISerializable
{
/// <summary>
Expand Down