(MQ cleanup) Remove size_t from managed Brotli code (#44043)
authorLevi Broderick <GrabYourPitchforks@users.noreply.github.com>
Mon, 2 Nov 2020 02:57:26 +0000 (18:57 -0800)
committerGitHub <noreply@github.com>
Mon, 2 Nov 2020 02:57:26 +0000 (21:57 -0500)
* 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>
src/libraries/Common/src/Interop/Interop.Brotli.cs
src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/dec/BrotliDecoder.cs
src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/enc/BrotliEncoder.cs

index c65c359..8052172 100644 (file)
@@ -5,7 +5,6 @@ using System;
 using System.Runtime.InteropServices;
 using System.IO.Compression;
 using Microsoft.Win32.SafeHandles;
-using size_t = System.IntPtr;
 
 internal static partial class Interop
 {
@@ -16,11 +15,11 @@ internal static partial class Interop
 
         [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);
@@ -36,8 +35,8 @@ internal static partial class Interop
 
         [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);
@@ -46,6 +45,6 @@ internal static partial class Interop
         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);
     }
 }
index 181afc5..384a27f 100644 (file)
@@ -5,7 +5,6 @@ using System.Buffers;
 using System.Diagnostics;
 using System.Runtime.InteropServices;
 using Microsoft.Win32.SafeHandles;
-using size_t = System.IntPtr;
 
 namespace System.IO.Compression
 {
@@ -49,11 +48,11 @@ namespace System.IO.Compression
             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)
@@ -61,11 +60,15 @@ namespace System.IO.Compression
                     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;
 
@@ -94,8 +97,11 @@ namespace System.IO.Compression
             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;
             }
index 870cdc0..55647b8 100644 (file)
@@ -5,7 +5,6 @@ using System.Buffers;
 using System.Diagnostics;
 using System.Runtime.InteropServices;
 using Microsoft.Win32.SafeHandles;
-using size_t = System.IntPtr;
 
 namespace System.IO.Compression
 {
@@ -125,11 +124,11 @@ namespace System.IO.Compression
 
             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)
@@ -137,14 +136,19 @@ namespace System.IO.Compression
                     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;
                         }
@@ -176,8 +180,11 @@ namespace System.IO.Compression
                 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;
                 }