Skip to content

Conversation

alinpahontu2912
Copy link
Member

Follow up PR adressing post-merge comments for #119089

@Copilot Copilot AI review requested due to automatic review settings September 25, 2025 09:41
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Sep 25, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR follows up on the previous changes to update MemoryStream's maximum capacity improvements by addressing post-merge comments. The primary purpose is to replace hardcoded int.MaxValue references with Array.MaxLength for better consistency and accuracy in memory stream capacity handling.

  • Replaced hardcoded int.MaxValue references with Array.MaxLength throughout MemoryStream implementation
  • Updated error messages to dynamically reference the actual maximum array length
  • Added comprehensive test coverage for capacity boundary checks

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/libraries/System.Private.CoreLib/src/System/IO/MemoryStream.cs Updated max length constant to use Array.MaxLength, added capacity validation, and improved error messages
src/libraries/System.Private.CoreLib/src/Resources/Strings.resx Updated error message template to accept dynamic maximum length parameter
src/libraries/System.Runtime/tests/System.IO.Tests/MemoryStream/MemoryStreamTests.cs Updated seek overflow tests and added new capacity boundary test
src/libraries/System.Runtime/tests/System.IO.Tests/BinaryWriter/BinaryWriterTests.cs Updated test array to use Array.MaxLength instead of int.MaxValue

@alinpahontu2912
Copy link
Member Author

Current failures don't seem related to these changes

@jkotas
Copy link
Member

jkotas commented Sep 27, 2025

/azp run runtime-libraries-coreclr outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@alinpahontu2912 alinpahontu2912 requested a review from a team September 29, 2025 10:17
@alinpahontu2912 alinpahontu2912 merged commit a531de6 into dotnet:main Oct 1, 2025
142 of 152 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants