Skip to content

Commit

Permalink
Merge pull request #327 from drewnoakes/reduce-bounds-checking
Browse files Browse the repository at this point in the history 
Remove excessive bounds checking in IndexedReader
  • Loading branch information
@drewnoakes
drewnoakes authored May 8, 2023
2 parents 0ac7d01 + 125eed5 commit 2fd3932
Show file tree
Hide file tree
Showing 12 changed files with 118 additions and 100 deletions.
3 changes: 1 addition & 2 deletions MetadataExtractor/IO/ByteArrayReader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,8 @@ public ByteArrayReader(byte[] buffer, int baseOffset = 0, bool isMotorolaByteOrd

public override long Length => _buffer.Length - _baseOffset;

public override byte GetByte(int index)
protected override byte GetByteInternal(int index)
{
ValidateIndex(index, 1);
return _buffer[index + _baseOffset];
}

Expand Down
22 changes: 15 additions & 7 deletions MetadataExtractor/IO/IndexedCapturingReader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,21 @@ protected override bool IsValidIndex(int index, int bytesRequested)

public override int ToUnshiftedOffset(int localOffset) => localOffset;

public override byte GetByte(int index)
private void GetPosition(int index, out int chunkIndex, out int innerIndex)
{
ValidateIndex(index, 1);
#if NET35 || NET45 || NETSTANDARD2_0
chunkIndex = Math.DivRem(index, _chunkLength, out innerIndex);
#elif NET5_0_OR_GREATER
(chunkIndex, innerIndex) = Math.DivRem(index, _chunkLength);
#else
chunkIndex = index / _chunkLength;
innerIndex = index % _chunkLength;
#endif
}

var chunkIndex = index / _chunkLength;
var innerIndex = index % _chunkLength;
protected override byte GetByteInternal(int index)
{
GetPosition(index, out int chunkIndex, out int innerIndex);
var chunk = _chunks[chunkIndex];
return chunk[innerIndex];
}
Expand All @@ -151,8 +160,7 @@ public override byte[] GetBytes(int index, int count)
var toIndex = 0;
while (remaining != 0)
{
var fromChunkIndex = fromIndex / _chunkLength;
var fromInnerIndex = fromIndex % _chunkLength;
GetPosition(fromIndex, out int fromChunkIndex, out int fromInnerIndex);
var length = Math.Min(remaining, _chunkLength - fromInnerIndex);
var chunk = _chunks[fromChunkIndex];
Array.Copy(chunk, fromInnerIndex, bytes, toIndex, length);
Expand Down Expand Up @@ -188,7 +196,7 @@ public ShiftedIndexedCapturingReader(IndexedCapturingReader baseReader, int base

public override int ToUnshiftedOffset(int localOffset) => localOffset + _baseOffset;

public override byte GetByte(int index) => _baseReader.GetByte(_baseOffset + index);
protected override byte GetByteInternal(int index) => _baseReader.GetByteInternal(_baseOffset + index);

public override byte[] GetBytes(int index, int count) => _baseReader.GetBytes(_baseOffset + index, count);

Expand Down
159 changes: 85 additions & 74 deletions MetadataExtractor/IO/IndexedReader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,11 @@ protected IndexedReader(bool isMotorolaByteOrder)
public abstract int ToUnshiftedOffset(int localOffset);

/// <summary>Gets the byte value at the specified byte <c>index</c>.</summary>
/// <remarks>
/// Implementations must validate <paramref name="index"/> by calling <see cref="ValidateIndex"/>.
/// </remarks>
/// <remarks>Implementations should assume <paramref name="index"/> has already been validated.</remarks>
/// <param name="index">The index from which to read the byte</param>
/// <returns>The read byte value</returns>
/// <exception cref="ArgumentException"><c>index</c> is negative</exception>
/// <exception cref="BufferBoundsException">if the requested byte is beyond the end of the underlying data source</exception>
/// <exception cref="System.IO.IOException">if the byte is unable to be read</exception>
public abstract byte GetByte(int index);
protected abstract byte GetByteInternal(int index);

/// <summary>Returns the required number of bytes from the specified index from the underlying source.</summary>
/// <param name="index">The index from which the bytes begins in the underlying source</param>
Expand Down Expand Up @@ -96,18 +92,33 @@ public bool GetBit(int index)
var byteIndex = index / 8;
var bitIndex = index % 8;
ValidateIndex(byteIndex, 1);
var b = GetByte(byteIndex);
var b = GetByteInternal(byteIndex);
return ((b >> bitIndex) & 1) == 1;
}

/// <summary>Gets the byte value at the specified byte <c>index</c>.</summary>
/// <remarks>
/// Implementations must validate <paramref name="index"/> by calling <see cref="ValidateIndex"/>.
/// </remarks>
/// <param name="index">The index from which to read the byte</param>
/// <returns>The read byte value</returns>
/// <exception cref="ArgumentException"><c>index</c> is negative</exception>
/// <exception cref="BufferBoundsException">if the requested byte is beyond the end of the underlying data source</exception>
/// <exception cref="System.IO.IOException">if the byte is unable to be read</exception>
public byte GetByte(int index)
{
ValidateIndex(index, 1);
return GetByteInternal(index);
}

/// <summary>Returns a signed 8-bit int calculated from one byte of data at the specified index.</summary>
/// <param name="index">position within the data buffer to read byte</param>
/// <returns>the 8 bit signed byte value</returns>
/// <exception cref="System.IO.IOException">the buffer does not contain enough bytes to service the request, or index is negative</exception>
public sbyte GetSByte(int index)
{
ValidateIndex(index, 1);
return unchecked((sbyte)GetByte(index));
return unchecked((sbyte)GetByteInternal(index));
}

#pragma warning disable format
Expand All @@ -123,13 +134,13 @@ public ushort GetUInt16(int index)
{
// Motorola - MSB first
return (ushort)
(GetByte(index ) << 8 |
GetByte(index + 1));
(GetByteInternal(index ) << 8 |
GetByteInternal(index + 1));
}
// Intel ordering - LSB first
return (ushort)
(GetByte(index + 1) << 8 |
GetByte(index ));
(GetByteInternal(index + 1) << 8 |
GetByteInternal(index ));
}

/// <summary>Returns a signed 16-bit int calculated from two bytes of data at the specified index (MSB, LSB).</summary>
Expand All @@ -143,13 +154,13 @@ public short GetInt16(int index)
{
// Motorola - MSB first
return (short)
(GetByte(index ) << 8 |
GetByte(index + 1));
(GetByteInternal(index ) << 8 |
GetByteInternal(index + 1));
}
// Intel ordering - LSB first
return (short)
(GetByte(index + 1) << 8 |
GetByte(index));
(GetByteInternal(index + 1) << 8 |
GetByteInternal(index));
}

/// <summary>Get a 24-bit unsigned integer from the buffer, returning it as an int.</summary>
Expand All @@ -163,15 +174,15 @@ public int GetInt24(int index)
{
// Motorola - MSB first (big endian)
return
GetByte(index ) << 16 |
GetByte(index + 1) << 8 |
GetByte(index + 2);
GetByteInternal(index ) << 16 |
GetByteInternal(index + 1) << 8 |
GetByteInternal(index + 2);
}
// Intel ordering - LSB first (little endian)
return
GetByte(index + 2) << 16 |
GetByte(index + 1) << 8 |
GetByte(index );
GetByteInternal(index + 2) << 16 |
GetByteInternal(index + 1) << 8 |
GetByteInternal(index );
}

/// <summary>Get a 32-bit unsigned integer from the buffer, returning it as a long.</summary>
Expand All @@ -185,17 +196,17 @@ public uint GetUInt32(int index)
{
// Motorola - MSB first (big endian)
return (uint)
(GetByte(index ) << 24 |
GetByte(index + 1) << 16 |
GetByte(index + 2) << 8 |
GetByte(index + 3));
(GetByteInternal(index ) << 24 |
GetByteInternal(index + 1) << 16 |
GetByteInternal(index + 2) << 8 |
GetByteInternal(index + 3));
}
// Intel ordering - LSB first (little endian)
return (uint)
(GetByte(index + 3) << 24 |
GetByte(index + 2) << 16 |
GetByte(index + 1) << 8 |
GetByte(index ));
(GetByteInternal(index + 3) << 24 |
GetByteInternal(index + 2) << 16 |
GetByteInternal(index + 1) << 8 |
GetByteInternal(index ));
}

/// <summary>Returns a signed 32-bit integer from four bytes of data at the specified index the buffer.</summary>
Expand All @@ -209,17 +220,17 @@ public int GetInt32(int index)
{
// Motorola - MSB first (big endian)
return
GetByte(index ) << 24 |
GetByte(index + 1) << 16 |
GetByte(index + 2) << 8 |
GetByte(index + 3);
GetByteInternal(index ) << 24 |
GetByteInternal(index + 1) << 16 |
GetByteInternal(index + 2) << 8 |
GetByteInternal(index + 3);
}
// Intel ordering - LSB first (little endian)
return
GetByte(index + 3) << 24 |
GetByte(index + 2) << 16 |
GetByte(index + 1) << 8 |
GetByte(index );
GetByteInternal(index + 3) << 24 |
GetByteInternal(index + 2) << 16 |
GetByteInternal(index + 1) << 8 |
GetByteInternal(index );
}

/// <summary>Get a signed 64-bit integer from the buffer.</summary>
Expand All @@ -233,25 +244,25 @@ public long GetInt64(int index)
{
// Motorola - MSB first
return
(long)GetByte(index ) << 56 |
(long)GetByte(index + 1) << 48 |
(long)GetByte(index + 2) << 40 |
(long)GetByte(index + 3) << 32 |
(long)GetByte(index + 4) << 24 |
(long)GetByte(index + 5) << 16 |
(long)GetByte(index + 6) << 8 |
GetByte(index + 7);
(long)GetByteInternal(index ) << 56 |
(long)GetByteInternal(index + 1) << 48 |
(long)GetByteInternal(index + 2) << 40 |
(long)GetByteInternal(index + 3) << 32 |
(long)GetByteInternal(index + 4) << 24 |
(long)GetByteInternal(index + 5) << 16 |
(long)GetByteInternal(index + 6) << 8 |
GetByteInternal(index + 7);
}
// Intel ordering - LSB first
return
(long)GetByte(index + 7) << 56 |
(long)GetByte(index + 6) << 48 |
(long)GetByte(index + 5) << 40 |
(long)GetByte(index + 4) << 32 |
(long)GetByte(index + 3) << 24 |
(long)GetByte(index + 2) << 16 |
(long)GetByte(index + 1) << 8 |
GetByte(index );
(long)GetByteInternal(index + 7) << 56 |
(long)GetByteInternal(index + 6) << 48 |
(long)GetByteInternal(index + 5) << 40 |
(long)GetByteInternal(index + 4) << 32 |
(long)GetByteInternal(index + 3) << 24 |
(long)GetByteInternal(index + 2) << 16 |
(long)GetByteInternal(index + 1) << 8 |
GetByteInternal(index );
}

/// <summary>Get an unsigned 64-bit integer from the buffer.</summary>
Expand All @@ -265,25 +276,25 @@ public ulong GetUInt64(int index)
{
// Motorola - MSB first
return
(ulong)GetByte(index ) << 56 |
(ulong)GetByte(index + 1) << 48 |
(ulong)GetByte(index + 2) << 40 |
(ulong)GetByte(index + 3) << 32 |
(ulong)GetByte(index + 4) << 24 |
(ulong)GetByte(index + 5) << 16 |
(ulong)GetByte(index + 6) << 8 |
GetByte(index + 7);
(ulong)GetByteInternal(index ) << 56 |
(ulong)GetByteInternal(index + 1) << 48 |
(ulong)GetByteInternal(index + 2) << 40 |
(ulong)GetByteInternal(index + 3) << 32 |
(ulong)GetByteInternal(index + 4) << 24 |
(ulong)GetByteInternal(index + 5) << 16 |
(ulong)GetByteInternal(index + 6) << 8 |
GetByteInternal(index + 7);
}
// Intel ordering - LSB first
return
(ulong)GetByte(index + 7) << 56 |
(ulong)GetByte(index + 6) << 48 |
(ulong)GetByte(index + 5) << 40 |
(ulong)GetByte(index + 4) << 32 |
(ulong)GetByte(index + 3) << 24 |
(ulong)GetByte(index + 2) << 16 |
(ulong)GetByte(index + 1) << 8 |
GetByte(index );
(ulong)GetByteInternal(index + 7) << 56 |
(ulong)GetByteInternal(index + 6) << 48 |
(ulong)GetByteInternal(index + 5) << 40 |
(ulong)GetByteInternal(index + 4) << 32 |
(ulong)GetByteInternal(index + 3) << 24 |
(ulong)GetByteInternal(index + 2) << 16 |
(ulong)GetByteInternal(index + 1) << 8 |
GetByteInternal(index );
}

#pragma warning restore format
Expand All @@ -299,15 +310,15 @@ public float GetS15Fixed16(int index)
ValidateIndex(index, 4);
if (IsMotorolaByteOrder)
{
float res = GetByte(index) << 8 | GetByte(index + 1);
var d = GetByte(index + 2) << 8 | GetByte(index + 3);
float res = GetByteInternal(index) << 8 | GetByteInternal(index + 1);
var d = GetByteInternal(index + 2) << 8 | GetByteInternal(index + 3);
return (float)(res + d / 65536.0);
}
else
{
// this particular branch is untested
var d = GetByte(index + 1) << 8 | GetByte(index);
float res = GetByte(index + 3) << 8 | GetByte(index + 2);
var d = GetByteInternal(index + 1) << 8 | GetByteInternal(index);
float res = GetByteInternal(index + 3) << 8 | GetByteInternal(index + 2);
return (float)(res + d / 65536.0);
}
}
Expand Down
2 changes: 1 addition & 1 deletion MetadataExtractor/IO/IndexedSeekingReader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public IndexedSeekingReader(Stream stream, int baseOffset = 0, bool isMotorolaBy

public override long Length { get; }

public override byte GetByte(int index)
protected override byte GetByteInternal(int index)
{
ValidateIndex(index, 1);

Expand Down
4 changes: 0 additions & 4 deletions MetadataExtractor/PublicAPI/net35/PublicAPI.Shipped.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ abstract MetadataExtractor.Formats.Tiff.DirectoryTiffHandler.HasFollowerIfd() ->
abstract MetadataExtractor.Formats.Tiff.DirectoryTiffHandler.ProcessTiffMarker(ushort marker) -> MetadataExtractor.Formats.Tiff.TiffStandard
abstract MetadataExtractor.Formats.Tiff.DirectoryTiffHandler.TryCustomProcessFormat(int tagId, MetadataExtractor.Formats.Tiff.TiffDataFormatCode formatCode, ulong componentCount, out ulong byteCount) -> bool
abstract MetadataExtractor.Formats.Tiff.DirectoryTiffHandler.TryEnterSubIfd(int tagType) -> bool
abstract MetadataExtractor.IO.IndexedReader.GetByte(int index) -> byte
abstract MetadataExtractor.IO.IndexedReader.GetBytes(int index, int count) -> byte[]!
abstract MetadataExtractor.IO.IndexedReader.IsValidIndex(int index, int bytesRequested) -> bool
abstract MetadataExtractor.IO.IndexedReader.Length.get -> long
Expand Down Expand Up @@ -4266,21 +4265,18 @@ override MetadataExtractor.Formats.Xmp.XmpDirectory.Name.get -> string!
override MetadataExtractor.GeoLocation.Equals(object? obj) -> bool
override MetadataExtractor.GeoLocation.GetHashCode() -> int
override MetadataExtractor.GeoLocation.ToString() -> string!
override MetadataExtractor.IO.ByteArrayReader.GetByte(int index) -> byte
override MetadataExtractor.IO.ByteArrayReader.GetBytes(int index, int count) -> byte[]!
override MetadataExtractor.IO.ByteArrayReader.IsValidIndex(int index, int bytesRequested) -> bool
override MetadataExtractor.IO.ByteArrayReader.Length.get -> long
override MetadataExtractor.IO.ByteArrayReader.ToUnshiftedOffset(int localOffset) -> int
override MetadataExtractor.IO.ByteArrayReader.ValidateIndex(int index, int bytesRequested) -> void
override MetadataExtractor.IO.ByteArrayReader.WithByteOrder(bool isMotorolaByteOrder) -> MetadataExtractor.IO.IndexedReader!
override MetadataExtractor.IO.ByteArrayReader.WithShiftedBaseOffset(int shift) -> MetadataExtractor.IO.IndexedReader!
override MetadataExtractor.IO.IndexedCapturingReader.GetByte(int index) -> byte
override MetadataExtractor.IO.IndexedCapturingReader.GetBytes(int index, int count) -> byte[]!
override MetadataExtractor.IO.IndexedCapturingReader.Length.get -> long
override MetadataExtractor.IO.IndexedCapturingReader.ToUnshiftedOffset(int localOffset) -> int
override MetadataExtractor.IO.IndexedCapturingReader.WithByteOrder(bool isMotorolaByteOrder) -> MetadataExtractor.IO.IndexedReader!
override MetadataExtractor.IO.IndexedCapturingReader.WithShiftedBaseOffset(int shift) -> MetadataExtractor.IO.IndexedReader!
override MetadataExtractor.IO.IndexedSeekingReader.GetByte(int index) -> byte
override MetadataExtractor.IO.IndexedSeekingReader.GetBytes(int index, int count) -> byte[]!
override MetadataExtractor.IO.IndexedSeekingReader.IsValidIndex(int index, int bytesRequested) -> bool
override MetadataExtractor.IO.IndexedSeekingReader.Length.get -> long
Expand Down
4 changes: 4 additions & 0 deletions MetadataExtractor/PublicAPI/net35/PublicAPI.Unshipped.txt
View file
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
#nullable enable
abstract MetadataExtractor.IO.IndexedReader.GetByteInternal(int index) -> byte
MetadataExtractor.IO.IndexedReader.GetByte(int index) -> byte
override MetadataExtractor.IO.ByteArrayReader.GetByteInternal(int index) -> byte
override MetadataExtractor.IO.IndexedSeekingReader.GetByteInternal(int index) -> byte
Loading

0 comments on commit 2fd3932

Please sign in to comment.