-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Extend FileStreamOptions with BufferSize, allow to specify 0 to disable the buffering #52928
Conversation
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
Tagging subscribers to this area: @carlossanlop Issue Details
|
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.
Otherwise LGTM, thanks.
src/libraries/System.Private.CoreLib/src/System/IO/FileStreamOptions.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.IO.FileSystem/tests/FileStream/FileStreamConformanceTests.cs
Show resolved
Hide resolved
Co-authored-by: David Cantú <dacantu@microsoft.com>
AssertExtensions.Throws<ArgumentOutOfRangeException>("bufferSize", () => CreateFileStream(GetTestFilePath(), FileMode.Create, FileAccess.ReadWrite, FileShare.Read, 0)); | ||
using (CreateFileStream(GetTestFilePath(), FileMode.Create, FileAccess.ReadWrite, FileShare.Read, 0)) | ||
{ | ||
} | ||
} |
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 think it'd be better to just change ValidBufferSize below to be a theory, with [InlineData(0)]
, [InlineData(64 * 1024)]
, and whatever other sizes we want to validate.
@@ -27,5 +27,10 @@ public sealed class FileStreamOptions | |||
/// In other cases (including the default 0 value), it's ignored. | |||
/// </summary> | |||
public long PreallocationSize { get; set; } | |||
/// <summary> | |||
/// The size of the buffer used by <see cref="FileStream" /> for buffering. The default buffer size is 4096. | |||
/// 0 or 1 means that buffering should be disabled. Negative values are not allowed. |
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.
Negative values are not allowed.
We don't want to do such validation in the setter?
#15088 (comment)