-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Fix ReadOnlySpanGetReferenceAndReadInteger on BigEndian #2 #86323
Conversation
Tagging subscribers to this area: @dotnet/area-system-memory Issue Detailsnull
|
/azp run runtime-community |
Azure Pipelines successfully started running 1 pipeline(s). |
@stephentoub @akoeplinger PTAL, I was too overconfident in my Big Endian skills in #85969 (comment)
|
ref MemoryMarshal.GetReference("hello world 1".AsSpan())), 0))); | ||
|
||
Assert.Equal(BitConverter.IsLittleEndian ? | ||
0x6F_00_6C_00_6C_00_65_00 : | ||
0x00_65_00_6C_00_6C_00_6F, | ||
Unsafe.As<byte, long>(ref Unsafe.Add(ref Unsafe.As<char, byte>( | ||
0x68_00_65_00_6C_00_6C_00, |
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'm not clear on why this is the right answer. Can you explain?
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.
My understanding is this:
// "hello world 2"
// LE: 68_00_65_00_6C_00_6C_00_6F_00_20_00_77_00_6F_00_72_00_6C_00_64_00_20_00_32_00
// 6F_00_6C_00_6C_00_65_00 (we read 8 bytes with offset = 1 bytes)
//
//
// BE: 00_68_00_65_00_6C_00_6C_00_6F_00_20_00_77_00_6F_00_72_00_6C_00_64_00_20_00_32
// 68_00_65_00_6C_00_6C_00 (we read 8 bytes with offset = 1 bytes)
//
Console.WriteLine(
BitConverter.ToString(
Encoding.Unicode.GetBytes("hello world 2")).Replace("-", "_"));
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.
Same picture for the int
case (that worked):
// "hello world 1"
// LE: 68_00_65_00_6C_00_6C_00_6F_00_20_00_77_00_6F_00_72_00_6C_00_64_00_20_00_31_00
// 00_65_00_68 (read 4 bytes)
//
//
// BE: 00_68_00_65_00_6C_00_6C_00_6F_00_20_00_77_00_6F_00_72_00_6C_00_64_00_20_00_31
// 00_68_00_65 (read 4 bytes)
so chars here have swapped order ('h' is 68_00 on LE and 00_68 on BE)
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 see. Thanks.
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.
@stephentoub can you approve then? 🙂
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.
GitHub wasn't letting me. Now it is. Done.
No description provided.