Skip to content

Fix StreamExtensions.Read<T> when using buffered streams#520

Merged
Sergio0694 merged 3 commits intomainfrom
dev/fix-stream-read-of-t
Dec 7, 2022
Merged

Fix StreamExtensions.Read<T> when using buffered streams#520
Sergio0694 merged 3 commits intomainfrom
dev/fix-stream-read-of-t

Conversation

@Sergio0694
Copy link
Copy Markdown
Member

Closes #513

PR Checklist

  • Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
  • Based off latest main branch of toolkit
  • PR doesn't include merge commits (always rebase on top of our main, if needed)
  • Tested code with current supported SDKs
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Header has been added to all new source files (run build/UpdateHeaders.bat)
  • Contains NO breaking changes
  • Every new API (including internal ones) has full XML docs
  • Code follows all style conventions

@Sergio0694 Sergio0694 added next preview ✈️ This changes will be available in the upcoming preview optimization ☄ Performance or memory usage improvements high-performance 🚂 Issues/PRs for the HighPerformance package bugfix 🔧 PRs fixing a discovered bug labels Dec 7, 2022
@Sergio0694 Sergio0694 merged commit 839fd64 into main Dec 7, 2022
@delete-merged-branch delete-merged-branch bot deleted the dev/fix-stream-read-of-t branch December 7, 2022 23:01
do
{
if (stream.Read(new Span<byte>(&result, length)) != length)
int bytesRead = stream.Read(new Span<byte>((byte*)&result + bytesOffset, sizeof(T) - bytesOffset));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For the .NET 7 target Stream.ReadExactly could be used. It boils down the be almost a one-liner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix 🔧 PRs fixing a discovered bug high-performance 🚂 Issues/PRs for the HighPerformance package next preview ✈️ This changes will be available in the upcoming preview optimization ☄ Performance or memory usage improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stream.Read<T>() makes incorrect assumptions about Stream.Read(Span)'s return value

3 participants