Remove IBinaryReader and revert related code changes.

Turns out the unification of `PEBinaryReader` and `BlobReader` can be avoided for now, and refactorings can still be made without it.

Author: teo-tsirpanis

src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj

@@ -27,7 +27,6 @@ The System.Reflection.Metadata library is built-in as part of the shared framewo
              Link="Common\Interop\Windows\kernel32\Interop.ReadFile_SafeHandle_IntPtr.cs"
              Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'" />
     <Compile Include="$(CoreLibSharedDir)System\Runtime\CompilerServices\IsExternalInit.cs" Link="System\Runtime\CompilerServices\IsExternalInit.cs" Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'" />
-    <Compile Include="System\Reflection\Internal\Utilities\IBinaryReader.cs" />
     <Compile Include="System\Reflection\Internal\Utilities\PinnedObject.cs" />
     <Compile Include="System\Reflection\Internal\Utilities\CriticalDisposableObject.cs" />
     <Compile Include="System\Reflection\Internal\Utilities\ExceptionUtilities.cs" />

src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/IBinaryReader.cs

@@ -11,7 +11,7 @@
 namespace System.Reflection.Metadata
 {
     [DebuggerDisplay("{GetDebuggerDisplay(),nq}")]
-    public unsafe struct BlobReader : IBinaryReader
+    public unsafe struct BlobReader
     {
         internal const int InvalidCompressedInteger = int.MaxValue;
 
@@ -438,16 +438,6 @@ public void ReadBytes(int byteCount, byte[] buffer, int bufferOffset)
             Marshal.Copy((IntPtr)GetCurrentPointerAndAdvance(byteCount), buffer, bufferOffset, byteCount);
         }
 
-        string IBinaryReader.ReadNullPaddedUTF8(int byteCount)
-        {
-            byte* ptr = GetCurrentPointerAndAdvance(byteCount);
-            while (byteCount > 0 && ptr[byteCount - 1] == 0)
-            {
-                byteCount--;
-            }
-            return Encoding.UTF8.GetString(ptr, byteCount);
-        }
-
         internal string ReadUtf8NullTerminated()
         {
             int bytesRead;

src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/BlobReader.cs

@@ -8,40 +8,40 @@ public sealed class CoffHeader
         /// <summary>
         /// The type of target machine.
         /// </summary>
-        public Machine Machine { get; private init; }
+        public Machine Machine { get; }
 
         /// <summary>
         /// The number of sections. This indicates the size of the section table, which immediately follows the headers.
         /// </summary>
-        public short NumberOfSections { get; private init; }
+        public short NumberOfSections { get; }
 
         /// <summary>
         /// The low 32 bits of the number of seconds since 00:00 January 1, 1970, that indicates when the file was created.
         /// </summary>
-        public int TimeDateStamp { get; private init; }
+        public int TimeDateStamp { get; }
 
         /// <summary>
         /// The file pointer to the COFF symbol table, or zero if no COFF symbol table is present.
         /// This value should be zero for a PE image.
         /// </summary>
-        public int PointerToSymbolTable { get; private init; }
+        public int PointerToSymbolTable { get; }
 
         /// <summary>
         /// The number of entries in the symbol table. This data can be used to locate the string table,
         /// which immediately follows the symbol table. This value should be zero for a PE image.
         /// </summary>
-        public int NumberOfSymbols { get; private init; }
+        public int NumberOfSymbols { get; }
 
         /// <summary>
         /// The size of the optional header, which is required for executable files but not for object files.
         /// This value should be zero for an object file.
         /// </summary>
-        public short SizeOfOptionalHeader { get; private init; }
+        public short SizeOfOptionalHeader { get; }
 
         /// <summary>
         /// The flags that indicate the attributes of the file.
         /// </summary>
-        public Characteristics Characteristics { get; private init; }
+        public Characteristics Characteristics { get; }
 
         internal const int Size =
             sizeof(short) + // Machine
@@ -52,20 +52,15 @@ public sealed class CoffHeader
             sizeof(short) + // SizeOfOptionalHeader:
             sizeof(ushort); // Characteristics
 
-        private CoffHeader() { }
-
-        internal static CoffHeader Create<TReader>(ref TReader reader) where TReader : IBinaryReader
+        internal CoffHeader(ref PEBinaryReader reader)
         {
-            return new CoffHeader()
-            {
-                Machine = (Machine)reader.ReadUInt16(),
-                NumberOfSections = reader.ReadInt16(),
-                TimeDateStamp = reader.ReadInt32(),
-                PointerToSymbolTable = reader.ReadInt32(),
-                NumberOfSymbols = reader.ReadInt32(),
-                SizeOfOptionalHeader = reader.ReadInt16(),
-                Characteristics = (Characteristics)reader.ReadUInt16(),
-            };
+            Machine = (Machine)reader.ReadUInt16();
+            NumberOfSections = reader.ReadInt16();
+            TimeDateStamp = reader.ReadInt32();
+            PointerToSymbolTable = reader.ReadInt32();
+            NumberOfSymbols = reader.ReadInt32();
+            SizeOfOptionalHeader = reader.ReadInt16();
+            Characteristics = (Characteristics)reader.ReadUInt16();
         }
     }
 }

src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/CoffHeader.cs

@@ -5,39 +5,34 @@ namespace System.Reflection.PortableExecutable
 {
     public sealed class CorHeader
     {
-        public ushort MajorRuntimeVersion { get; private init; }
-        public ushort MinorRuntimeVersion { get; private init; }
-        public DirectoryEntry MetadataDirectory { get; private init; }
-        public CorFlags Flags { get; private init; }
-        public int EntryPointTokenOrRelativeVirtualAddress { get; private init; }
-        public DirectoryEntry ResourcesDirectory { get; private init; }
-        public DirectoryEntry StrongNameSignatureDirectory { get; private init; }
-        public DirectoryEntry CodeManagerTableDirectory { get; private init; }
-        public DirectoryEntry VtableFixupsDirectory { get; private init; }
-        public DirectoryEntry ExportAddressTableJumpsDirectory { get; private init; }
-        public DirectoryEntry ManagedNativeHeaderDirectory { get; private init; }
+        public ushort MajorRuntimeVersion { get; }
+        public ushort MinorRuntimeVersion { get; }
+        public DirectoryEntry MetadataDirectory { get; }
+        public CorFlags Flags { get; }
+        public int EntryPointTokenOrRelativeVirtualAddress { get; }
+        public DirectoryEntry ResourcesDirectory { get; }
+        public DirectoryEntry StrongNameSignatureDirectory { get; }
+        public DirectoryEntry CodeManagerTableDirectory { get; }
+        public DirectoryEntry VtableFixupsDirectory { get; }
+        public DirectoryEntry ExportAddressTableJumpsDirectory { get; }
+        public DirectoryEntry ManagedNativeHeaderDirectory { get; }
 
-        private CorHeader() { }
-
-        internal static CorHeader Create<TReader>(ref TReader reader) where TReader : IBinaryReader
+        internal CorHeader(ref PEBinaryReader reader)
         {
             // byte count
             reader.ReadInt32();
 
-            return new CorHeader()
-            {
-                MajorRuntimeVersion = reader.ReadUInt16(),
-                MinorRuntimeVersion = reader.ReadUInt16(),
-                MetadataDirectory = DirectoryEntry.Create(ref reader),
-                Flags = (CorFlags)reader.ReadUInt32(),
-                EntryPointTokenOrRelativeVirtualAddress = reader.ReadInt32(),
-                ResourcesDirectory = DirectoryEntry.Create(ref reader),
-                StrongNameSignatureDirectory = DirectoryEntry.Create(ref reader),
-                CodeManagerTableDirectory = DirectoryEntry.Create(ref reader),
-                VtableFixupsDirectory = DirectoryEntry.Create(ref reader),
-                ExportAddressTableJumpsDirectory = DirectoryEntry.Create(ref reader),
-                ManagedNativeHeaderDirectory = DirectoryEntry.Create(ref reader),
-            };
+            MajorRuntimeVersion = reader.ReadUInt16();
+            MinorRuntimeVersion = reader.ReadUInt16();
+            MetadataDirectory = new DirectoryEntry(ref reader);
+            Flags = (CorFlags)reader.ReadUInt32();
+            EntryPointTokenOrRelativeVirtualAddress = reader.ReadInt32();
+            ResourcesDirectory = new DirectoryEntry(ref reader);
+            StrongNameSignatureDirectory = new DirectoryEntry(ref reader);
+            CodeManagerTableDirectory = new DirectoryEntry(ref reader);
+            VtableFixupsDirectory = new DirectoryEntry(ref reader);
+            ExportAddressTableJumpsDirectory = new DirectoryEntry(ref reader);
+            ManagedNativeHeaderDirectory = new DirectoryEntry(ref reader);
         }
     }
 }

src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/CorHeader.cs

@@ -14,9 +14,10 @@ public DirectoryEntry(int relativeVirtualAddress, int size)
             Size = size;
         }
 
-        internal static DirectoryEntry Create<TReader>(ref TReader reader) where TReader : IBinaryReader
+        internal DirectoryEntry(ref PEBinaryReader reader)
         {
-            return new DirectoryEntry(reader.ReadInt32(), reader.ReadInt32());
+            RelativeVirtualAddress = reader.ReadInt32();
+            Size = reader.ReadInt32();
         }
     }
 }

src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/DirectoryEntry.cs

@@ -17,7 +17,7 @@ namespace System.Reflection.PortableExecutable
     ///
     /// Only methods that are needed to read PE headers are implemented.
     /// </summary>
-    internal readonly struct PEBinaryReader : IBinaryReader
+    internal readonly struct PEBinaryReader
     {
         private readonly long _startOffset;
         private readonly long _maxOffset;
@@ -84,6 +84,14 @@ public ulong ReadUInt64()
             return _reader.ReadUInt64();
         }
 
+        /// <summary>
+        /// Reads a fixed-length byte block as a null-padded UTF-8 encoded string.
+        /// The padding is not included in the returned string.
+        ///
+        /// Note that it is legal for UTF-8 strings to contain NUL; if NUL occurs
+        /// between non-NUL codepoints, it is not considered to be padding and
+        /// is included in the result.
+        /// </summary>
         public string ReadNullPaddedUTF8(int byteCount)
         {
             byte[] bytes = ReadBytes(byteCount);