Invalid check in CheckedFile.cpp readPhysicalPage
asmaloney opened this issue · 6 comments
In PR #22 - CheckedFile.cpp:
bufView_->read(page_buffer, physicalPageSize);
size_t result = physicalPageSize;
if ( result < 0 || static_cast<size_t>(result) != physicalPageSize )
{
throw E57_EXCEPTION2(E57_ERROR_READ_FAILED, "fileName=" + fileName_ + " result=" + toString(result));
}
physicalPageSize
is a constant, so result
isn't a result.
(Also checking a size_t
for < 0 is meaningless.)
@jean-noelp What did you mean here?
You're right, this test doesn't make sense anymore.
Since this is a buffer now, reading operation should not be able to fail anyway.
What if the seek
right before this code fails?
Then it will set the buffer's cursorStream_
to the size of the buffer and read
will start reading there...
If the seek fails, an exception will be throwed in lseek64 at the line 478.
Also the user will have to seek at the beginning, or to close the file.
Normally this is the same behavior as before.
...right. And this is why I hate exceptions :-)
I'll remove that extra code mentioned above.
I understand.
If you want to have explicit exceptions, I suggest you to mention throwable functions like this:
https://en.cppreference.com/w/cpp/language/except_spec
Thanks. I would prefer to go the other way and get rid of them entirely, but I'm not that invested in this code :-)