-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
[C#] Fix truncated ArraySegment<byte> if elementSize != 1 #6462
Conversation
ping @dbaileychess |
@@ -112,7 +112,7 @@ public int __vector(int offset) | |||
// Get the data of a vector whoses offset is stored at "offset" in this object as an | |||
// ArraySegment<byte>. If the vector is not present in the ByteBuffer, | |||
// then a null value will be returned. | |||
public ArraySegment<byte>? __vector_as_arraysegment(int offset) | |||
public ArraySegment<byte>? __vector_as_arraysegment(int offset, int elementSize) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we're returning ArraySegment<byte>
instead of ArraySegment<T>
because of endianness? Though possibly this API would be more useful if we returned ArraySegment<T>
and asserted we're on little endian? Or is it not possible in C# to make a ArraySegment<T>
out of bytes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly by some unsafe code tricks but normally you are not allowed to cast no.
The relatively new MemoryMarshal class allows to cast Span
but curiously not Memory
, as of yet I fail to understand why. I also have not fully understood if use of MemoryMarshal is as unsafe as unsafe code but dotnet/runtime#41418 indicates it is equivalent to unsafe in many cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, then this can be merged if @dbaileychess doesn't have any comments.
)" This reverts commit cbbbaa6.
Intends to fix #6463.