Skip to content

Commit

Permalink
Remove excessive bounds checking in IndexedReader
Browse files Browse the repository at this point in the history 
Fixes #62

Adds a new abstract overload for reading a single byte. That method will not do any explicit bounds validation, so callers must do that beforehand. In cases where multiple bytes are read, the validation can be performed once per value, rather than once per byte.
  • Loading branch information
@drewnoakes
drewnoakes committed Jan 24, 2023
1 parent ffdd621 commit a5affc4
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(index, 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 @@ -1591,7 +1591,6 @@ abstract MetadataExtractor.Formats.Riff.RiffHandler.ShouldAcceptList(string! fou
abstract MetadataExtractor.Formats.Riff.RiffHandler.ShouldAcceptRiffIdentifier(string! identifier) -> bool
abstract MetadataExtractor.Formats.Tiff.DirectoryTiffHandler.HasFollowerIfd() -> 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 @@ -3740,21 +3739,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
Loading

0 comments on commit a5affc4

Please sign in to comment.