Enhance DWARF memory reader with bounds checking and exception handli…#1167
Enhance DWARF memory reader with bounds checking and exception handli…#1167
Conversation
…ng for read operations
| reader.Peek().Should().Be(0x42); | ||
| reader.Position.Should().Be(0); | ||
| } | ||
| } |
There was a problem hiding this comment.
Does this file actually compile? :)
|
|
||
| private void EnsureAvailable(uint bytesToRead) | ||
| { | ||
| if (bytesToRead > (uint)Data.Length || Position > (uint)Data.Length - bytesToRead) |
There was a problem hiding this comment.
First part in front of || seems redundant (it's equivalent of second part, with Position = 0)
| { | ||
| if (bytesToRead > (uint)Data.Length || Position > (uint)Data.Length - bytesToRead) | ||
| { | ||
| throw new InvalidOperationException("Attempted to read past end of DWARF data buffer."); |
There was a problem hiding this comment.
Thinking if we should not set position to the end of the data as well. To make sure no loop tries to re-read that broken sequence (if they would not handle the exception correctly)
There was a problem hiding this comment.
Also, would it be worth having our custom exception for this scenario?
| if (Position >= Data.Length) | ||
| { | ||
| throw new InvalidOperationException("Attempted to read past end of DWARF data buffer."); | ||
| } |
There was a problem hiding this comment.
This seems to be redundant with the check in first iteration of while
| if (Position >= Data.Length) | ||
| { | ||
| Position = startPosition; | ||
| throw new InvalidOperationException("Attempted to read past end of DWARF data buffer."); | ||
| } |
There was a problem hiding this comment.
This is also redundant - it does the same check as last iteration of the while above
| @@ -224,17 +265,45 @@ public ulong ULEB128() | |||
| /// </summary> | |||
| public uint SLEB128() | |||
There was a problem hiding this comment.
Same questions as for ULEB128 above
| @@ -224,17 +265,45 @@ public ulong ULEB128() | |||
| /// </summary> | |||
| public uint SLEB128() | |||
There was a problem hiding this comment.
Same questions as for ULEB128 above
| @@ -83,6 +92,7 @@ public byte Peek() | |||
| /// <typeparam name="T">Type of the structure to be read</typeparam> | |||
| public T ReadStructure<T>() | |||
There was a problem hiding this comment.
This one is missing test
2 participants
…ng for read operations