Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use throw helpers in a few more places #82533

Merged
merged 3 commits into from
Feb 23, 2023
Merged

Use throw helpers in a few more places #82533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a56b721
Use throw helpers in a few more places
stephentoub Feb 23, 2023
39989d8
Update src/libraries/System.Private.CoreLib/src/System/String.cs
stephentoub Feb 23, 2023
3298c13
Add missing cast
stephentoub Feb 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 1 addition & 4 deletions src/libraries/System.Formats.Tar/src/Resources/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,6 @@
<data name="IO_SharingViolation_NoFileName" xml:space="preserve">
<value>The process cannot access the file because it is being used by another process.</value>
</data>
<data name="IO_StreamDisposed" xml:space="preserve">
<value>Cannot access a disposed stream.</value>
</data>
<data name="IO_StreamNotEmpty" xml:space="preserve">
<value>The stream is not empty.</value>
</data>
Expand Down Expand Up @@ -267,4 +264,4 @@
<data name="TarSizeFieldTooLargeForEntryFormat" xml:space="preserve">
<value>The value of the size field for the current entry of format '{0}' is greater than the format allows.</value>
</data>
</root>
</root>
5 changes: 1 addition & 4 deletions src/libraries/System.Formats.Tar/src/System/Formats/Tar/SubReadStream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,7 @@ internal bool HasReachedEnd

protected void ThrowIfDisposed()
{
if (_isDisposed)
{
throw new ObjectDisposedException(GetType().ToString(), SR.IO_StreamDisposed);
}
ObjectDisposedException.ThrowIf(_isDisposed, this);
}

private void ThrowIfBeyondEndOfStream()
Expand Down
3 changes: 0 additions & 3 deletions src/libraries/System.IO.Compression.Brotli/src/Resources/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,6 @@
<data name="ArgumentOutOfRange_Enum" xml:space="preserve">
<value>Enum value was out of legal range.</value>
</data>
<data name="ObjectDisposed_StreamClosed" xml:space="preserve">
<value>Cannot access a closed stream.</value>
</data>
<data name="ArgumentOutOfRange_NeedPosNum" xml:space="preserve">
<value>Positive number required.</value>
</data>
Expand Down
3 changes: 1 addition & 2 deletions src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliStream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ public BrotliStream(Stream stream, CompressionMode mode, bool leaveOpen)

private void EnsureNotDisposed()
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
{
if (_stream == null)
throw new ObjectDisposedException(GetType().Name, SR.ObjectDisposed_StreamClosed);
ObjectDisposedException.ThrowIf(_stream is null, this);
}

/// <summary>Releases the unmanaged resources used by the <see cref="System.IO.Compression.BrotliStream" /> and optionally releases the managed resources.</summary>
Expand Down
10 changes: 6 additions & 4 deletions src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/enc/BrotliEncoder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,14 @@ internal void SetWindow(int window)
/// <exception cref="System.ArgumentOutOfRangeException"><paramref name="inputSize" /> is less than 0, the minimum allowed input size, or greater than <see cref="int.MaxValue" /> - 515, the maximum allowed input size.</exception>
public static int GetMaxCompressedLength(int inputSize)
{
if (inputSize < 0 || inputSize > BrotliUtils.MaxInputSize)
{
throw new ArgumentOutOfRangeException(nameof(inputSize));
}
ArgumentOutOfRangeException.ThrowIfNegative(inputSize);
ArgumentOutOfRangeException.ThrowIfGreaterThan(inputSize, BrotliUtils.MaxInputSize);

if (inputSize == 0)
{
return 1;
}

int numLargeBlocks = inputSize >> 24;
int tail = inputSize & 0xFFFFFF;
int tailOverhead = (tail > (1 << 20)) ? 4 : 3;
Expand Down
3 changes: 0 additions & 3 deletions src/libraries/System.IO.Compression/src/Resources/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,6 @@
<data name="NotSupported_UnwritableStream" xml:space="preserve">
<value>Stream does not support writing.</value>
</data>
<data name="ObjectDisposed_StreamClosed" xml:space="preserve">
<value>Cannot access a closed stream.</value>
</data>
<data name="UnknownBlockType" xml:space="preserve">
<value>Unknown block type. Stream might be corrupted.</value>
</data>
Expand Down
8 changes: 1 addition & 7 deletions ...es/System.IO.Compression/src/System/IO/Compression/DeflateManaged/DeflateManagedStream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,7 @@ public override int ReadByte()

private void EnsureNotDisposed()
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
{
if (_stream == null)
ThrowStreamClosedException();
}

private static void ThrowStreamClosedException()
{
throw new ObjectDisposedException(nameof(DeflateStream), SR.ObjectDisposed_StreamClosed);
ObjectDisposedException.ThrowIf(_stream is null, this);
}

public override IAsyncResult BeginRead(byte[] buffer, int offset, int count, AsyncCallback? asyncCallback, object? asyncState) =>
Expand Down
6 changes: 1 addition & 5 deletions src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/DeflateStream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,11 +326,7 @@ internal int ReadCore(Span<byte> buffer)

private void EnsureNotDisposed()
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
{
if (_stream == null)
ThrowStreamClosedException();

static void ThrowStreamClosedException() =>
throw new ObjectDisposedException(nameof(DeflateStream), SR.ObjectDisposed_StreamClosed);
ObjectDisposedException.ThrowIf(_stream is null, this);
}

private void EnsureDecompressionMode()
Expand Down
10 changes: 1 addition & 9 deletions src/libraries/System.IO.Compression/src/System/IO/Compression/GZipStream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,15 +242,7 @@ public override Task CopyToAsync(Stream destination, int bufferSize, Cancellatio

private void CheckDeflateStream()
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
{
if (_deflateStream == null)
{
ThrowStreamClosedException();
}
}

private static void ThrowStreamClosedException()
{
throw new ObjectDisposedException(nameof(GZipStream), SR.ObjectDisposed_StreamClosed);
ObjectDisposedException.ThrowIf(_deflateStream is null, this);
}
}
}
10 changes: 1 addition & 9 deletions src/libraries/System.IO.Compression/src/System/IO/Compression/ZLibStream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,15 +279,7 @@ public override ValueTask DisposeAsync()
/// <summary>Throws an <see cref="ObjectDisposedException"/> if the stream is closed.</summary>
private void ThrowIfClosed()
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
{
if (_deflateStream is null)
{
ThrowClosedException();
}
ObjectDisposedException.ThrowIf(_deflateStream is null, this);
}

/// <summary>Throws an <see cref="ObjectDisposedException"/>.</summary>
[DoesNotReturn]
private static void ThrowClosedException() =>
throw new ObjectDisposedException(nameof(ZLibStream), SR.ObjectDisposed_StreamClosed);
}
}
5 changes: 1 addition & 4 deletions src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,10 +247,7 @@ private static FileAccess GetFileAccessFromRights(FileSystemRights rights)
access |= FileAccess.Write;
}

if (access == 0)
{
throw new ArgumentOutOfRangeException(nameof(rights));
}
ArgumentOutOfRangeException.ThrowIfZero((int)access, nameof(rights));

return access;
}
Expand Down
3 changes: 0 additions & 3 deletions src/libraries/System.Net.Sockets/src/Resources/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,9 +273,6 @@
<data name="NotSupported_UnwritableStream" xml:space="preserve">
<value>Stream does not support writing.</value>
</data>
<data name="ObjectDisposed_StreamClosed" xml:space="preserve">
<value>Cannot access a closed stream.</value>
</data>
<data name="ArgumentOutOfRange_PathLengthInvalid" xml:space="preserve">
<value>The path '{0}' is of an invalid length for use with domain sockets on this platform. The length must be between 1 and {1} characters, inclusive.</value>
</data>
Expand Down
10 changes: 8 additions & 2 deletions src/libraries/System.Private.CoreLib/src/System/String.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,13 +295,19 @@ private static unsafe string Ctor(ReadOnlySpan<char> value)

public static string Create<TState>(int length, TState state, SpanAction<char, TState> action)
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
{
ArgumentNullException.ThrowIfNull(action);
if (action is null)
Copy link
Contributor

@drieseng drieseng Feb 25, 2023 •
edited

Choose a reason for hiding this comment

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

@stephentoub: Just out of curiosity, why was this changed?

PS. I should sleep more :(

Copy link
Member Author

Choose a reason for hiding this comment

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

This pattern results in ever so slightly smaller asm, and as it's in a generic method and we already had the helper and the appropriately named constant argument and I was already editing the method, it was trivial to switch.

{
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.action);
}

if (length <= 0)
{
if (length == 0)
{
return Empty;
throw new ArgumentOutOfRangeException(nameof(length));
}

ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.length);
}

string result = FastAllocateString(length);
Expand Down
3 changes: 1 addition & 2 deletions src/mono/System.Private.CoreLib/src/System/GC.Mono.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,7 @@ public static void KeepAlive(object? obj)
public static int GetGeneration(WeakReference wo)
{
object? obj = wo.Target;
if (obj == null)
throw new ArgumentException(null, nameof(wo));
ArgumentNullException.ThrowIfNull(obj, nameof(wo));
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
return GetGeneration(obj);
}

Expand Down