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
src/Test.UnitTests.BinaryParsers/Dwarf/DwarfMemoryReaderTests.cs
Outdated
Show resolved
Hide resolved
|
|
||
| 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)
There was a problem hiding this comment.
Even if Position is 0, there might be a scenario where we want to read more bytes than length (Data.Length = 4 and bytesToRead = 8 -> that is the kind of scenario where first part is necessary
| { | ||
| 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?
There was a problem hiding this comment.
Well I put Position into exception so it would not make sense to change that
…ean up using directives
… DWARF memory operations
2 participants
…ng for read operations