path.d: add overflow checks #4713
Conversation
WalterBright
force-pushed
the
path-overflow
branch
from
1878d4e to
c6f706b
Compare
|
This uncovered a latent bug, too. |
| if (errno != ERANGE) | ||
| if (errno != ERANGE && | ||
| // On FreeBSD and OSX, errno can be left at 0 instead of set to ERANGE | ||
| errno != 0) |
There was a problem hiding this comment.
Shouldn't this check come inside the if clause above? I mean, I didn't think errno had any specified meaning if your return from the syscall was 0. It's always "if this returns nonzero, check errno".
There was a problem hiding this comment.
The idea is to have a "not enough memory" error result in increasing the memory size, and that is different from some other error.
|
ping @WalterBright Can we move forward with this PR please? |
| import core.checkedint : mulu; | ||
| bool overflow; | ||
| extra_memory_size = mulu(extra_memory_size, 2, overflow); | ||
| if (overflow) assert(0); |
There was a problem hiding this comment.
I was going to leave a comment about how there should be an assert message, but is the message even printed when using assert(0)?
There was a problem hiding this comment.
The message is printed if not built with -release, because in that case, assert(0) is compiled in like any assertion would be and throws an AssertError. However, with -release, when assertions are compiled out, it becomes a HLT instruction, and there is no message.
There was a problem hiding this comment.
Is there a better solution here than just halting the program when there's an overflow? We're talking about people's programs potentially crashing and there being no indication as to what happened.
There was a problem hiding this comment.
Well, you could throw an exception, or you could throw a static error singleton as Andrei has proposed before. Both come with their own problems.
There was a problem hiding this comment.
Andrei kind of halted Walter's addition of overflow checks with his checked int module because he believed that was the right way to do things. I don't really see why these need to be stopped in the mean time though
WalterBright
force-pushed
the
path-overflow
branch
from
c6f706b to
c0403e1
Compare
9 participants
No description provided.