Skip to content

Commit

Permalink
(MQ cleanup) Remove size_t from managed Brotli code (#44043)
Browse files Browse the repository at this point in the history
* Remove size_t from managed Brotli code

* Apply suggestions from code review

Co-authored-by: Stephen Toub <stoub@microsoft.com>

Co-authored-by: Stephen Toub <stoub@microsoft.com>
  • Loading branch information
GrabYourPitchforks and stephentoub authored Nov 2, 2020
1 parent e10f771 commit ecd979c
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 22 deletions.
13 changes: 6 additions & 7 deletions src/libraries/Common/src/Interop/Interop.Brotli.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Runtime.InteropServices;
using System.IO.Compression;
using Microsoft.Win32.SafeHandles;
using size_t = System.IntPtr;

internal static partial class Interop
{
Expand All @@ -16,11 +15,11 @@ internal static class Brotli

[DllImport(Libraries.CompressionNative)]
internal static extern unsafe int BrotliDecoderDecompressStream(
SafeBrotliDecoderHandle state, ref size_t availableIn, byte** nextIn,
ref size_t availableOut, byte** nextOut, out size_t totalOut);
SafeBrotliDecoderHandle state, ref nuint availableIn, byte** nextIn,
ref nuint availableOut, byte** nextOut, out nuint totalOut);

[DllImport(Libraries.CompressionNative)]
internal static extern unsafe bool BrotliDecoderDecompress(size_t availableInput, byte* inBytes, ref size_t availableOutput, byte* outBytes);
internal static extern unsafe bool BrotliDecoderDecompress(nuint availableInput, byte* inBytes, ref nuint availableOutput, byte* outBytes);

[DllImport(Libraries.CompressionNative)]
internal static extern void BrotliDecoderDestroyInstance(IntPtr state);
Expand All @@ -36,8 +35,8 @@ internal static extern unsafe int BrotliDecoderDecompressStream(

[DllImport(Libraries.CompressionNative)]
internal static extern unsafe bool BrotliEncoderCompressStream(
SafeBrotliEncoderHandle state, BrotliEncoderOperation op, ref size_t availableIn,
byte** nextIn, ref size_t availableOut, byte** nextOut, out size_t totalOut);
SafeBrotliEncoderHandle state, BrotliEncoderOperation op, ref nuint availableIn,
byte** nextIn, ref nuint availableOut, byte** nextOut, out nuint totalOut);

[DllImport(Libraries.CompressionNative)]
internal static extern bool BrotliEncoderHasMoreOutput(SafeBrotliEncoderHandle state);
Expand All @@ -46,6 +45,6 @@ internal static extern unsafe bool BrotliEncoderCompressStream(
internal static extern void BrotliEncoderDestroyInstance(IntPtr state);

[DllImport(Libraries.CompressionNative)]
internal static extern unsafe bool BrotliEncoderCompress(int quality, int window, int v, size_t availableInput, byte* inBytes, ref size_t availableOutput, byte* outBytes);
internal static extern unsafe bool BrotliEncoderCompress(int quality, int window, int v, nuint availableInput, byte* inBytes, ref nuint availableOutput, byte* outBytes);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Diagnostics;
using System.Runtime.InteropServices;
using Microsoft.Win32.SafeHandles;
using size_t = System.IntPtr;

namespace System.IO.Compression
{
Expand Down Expand Up @@ -49,23 +48,27 @@ public OperationStatus Decompress(ReadOnlySpan<byte> source, Span<byte> destinat
bytesWritten = 0;
if (Interop.Brotli.BrotliDecoderIsFinished(_state))
return OperationStatus.Done;
size_t availableOutput = (size_t)destination.Length;
size_t availableInput = (size_t)source.Length;
nuint availableOutput = (nuint)destination.Length;
nuint availableInput = (nuint)source.Length;
unsafe
{
// We can freely cast between int and size_t for two reasons:
// We can freely cast between int and nuint (.NET size_t equivalent) for two reasons:
// 1. Interop Brotli functions will always return an availableInput/Output value lower or equal to the one passed to the function
// 2. Span's have a maximum length of the int boundary.
while ((int)availableOutput > 0)
{
fixed (byte* inBytes = &MemoryMarshal.GetReference(source))
fixed (byte* outBytes = &MemoryMarshal.GetReference(destination))
{
int brotliResult = Interop.Brotli.BrotliDecoderDecompressStream(_state, ref availableInput, &inBytes, ref availableOutput, &outBytes, out size_t totalOut);
int brotliResult = Interop.Brotli.BrotliDecoderDecompressStream(_state, ref availableInput, &inBytes, ref availableOutput, &outBytes, out _);
if (brotliResult == 0) // Error
{
return OperationStatus.InvalidData;
}

Debug.Assert(availableInput <= (nuint)source.Length);
Debug.Assert(availableOutput <= (nuint)destination.Length);

bytesConsumed += source.Length - (int)availableInput;
bytesWritten += destination.Length - (int)availableOutput;

Expand Down Expand Up @@ -94,8 +97,11 @@ public static unsafe bool TryDecompress(ReadOnlySpan<byte> source, Span<byte> de
fixed (byte* inBytes = &MemoryMarshal.GetReference(source))
fixed (byte* outBytes = &MemoryMarshal.GetReference(destination))
{
size_t availableOutput = (size_t)destination.Length;
bool success = Interop.Brotli.BrotliDecoderDecompress((size_t)source.Length, inBytes, ref availableOutput, outBytes);
nuint availableOutput = (nuint)destination.Length;
bool success = Interop.Brotli.BrotliDecoderDecompress((nuint)source.Length, inBytes, ref availableOutput, outBytes);

Debug.Assert(success ? availableOutput <= (nuint)destination.Length : availableOutput == 0);

bytesWritten = (int)availableOutput;
return success;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Diagnostics;
using System.Runtime.InteropServices;
using Microsoft.Win32.SafeHandles;
using size_t = System.IntPtr;

namespace System.IO.Compression
{
Expand Down Expand Up @@ -125,26 +124,31 @@ internal OperationStatus Compress(ReadOnlySpan<byte> source, Span<byte> destinat

bytesWritten = 0;
bytesConsumed = 0;
size_t availableOutput = (size_t)destination.Length;
size_t availableInput = (size_t)source.Length;
nuint availableOutput = (nuint)destination.Length;
nuint availableInput = (nuint)source.Length;
unsafe
{
// We can freely cast between int and size_t for two reasons:
// We can freely cast between int and nuint (.NET size_t equivalent) for two reasons:
// 1. Interop Brotli functions will always return an availableInput/Output value lower or equal to the one passed to the function
// 2. Span's have a maximum length of the int boundary.
while ((int)availableOutput > 0)
{
fixed (byte* inBytes = &MemoryMarshal.GetReference(source))
fixed (byte* outBytes = &MemoryMarshal.GetReference(destination))
{
if (!Interop.Brotli.BrotliEncoderCompressStream(_state, operation, ref availableInput, &inBytes, ref availableOutput, &outBytes, out size_t totalOut))
if (!Interop.Brotli.BrotliEncoderCompressStream(_state, operation, ref availableInput, &inBytes, ref availableOutput, &outBytes, out _))
{
return OperationStatus.InvalidData;
}

Debug.Assert(availableInput <= (nuint)source.Length);
Debug.Assert(availableOutput <= (nuint)destination.Length);

bytesConsumed += source.Length - (int)availableInput;
bytesWritten += destination.Length - (int)availableOutput;

// no bytes written, no remaining input to give to the encoder, and no output in need of retrieving means we are Done
if ((int)availableOutput == destination.Length && !Interop.Brotli.BrotliEncoderHasMoreOutput(_state) && (int)availableInput == 0)
if ((int)availableOutput == destination.Length && !Interop.Brotli.BrotliEncoderHasMoreOutput(_state) && availableInput == 0)
{
return OperationStatus.Done;
}
Expand Down Expand Up @@ -176,8 +180,11 @@ public static bool TryCompress(ReadOnlySpan<byte> source, Span<byte> destination
fixed (byte* inBytes = &MemoryMarshal.GetReference(source))
fixed (byte* outBytes = &MemoryMarshal.GetReference(destination))
{
size_t availableOutput = (size_t)destination.Length;
bool success = Interop.Brotli.BrotliEncoderCompress(quality, window, /*BrotliEncoderMode*/ 0, (size_t)source.Length, inBytes, ref availableOutput, outBytes);
nuint availableOutput = (nuint)destination.Length;
bool success = Interop.Brotli.BrotliEncoderCompress(quality, window, /*BrotliEncoderMode*/ 0, (nuint)source.Length, inBytes, ref availableOutput, outBytes);

Debug.Assert(success ? availableOutput <= (nuint)destination.Length : availableOutput == 0);

bytesWritten = (int)availableOutput;
return success;
}
Expand Down

0 comments on commit ecd979c

Please sign in to comment.