Conversation
main/streams/memory.c
Outdated
| case SEEK_CUR: | ||
| if (offset < 0) { | ||
| if (ms->fpos < (size_t)(-offset)) { | ||
| if (ms->fpos < -(size_t)offset) { |
There was a problem hiding this comment.
The error goes away, but it's not really correct. The - effectively becomes a no-op. However, the intent here is to compare ms->fpos to a positive offset.
I'm not sure if we need this check at all? For positive offset, we're not checking for overflows and rely on eof checks when reading from the stream, so do we need to check for underflows for negative offset? It would simply wrap around the same way positive offsets do. Though I suppose removing this check is a subtle breaking change.
There was a problem hiding this comment.
I changed it as before.
case SEEK_CUR:
if (offset < 0) {
- if (ms->fpos < -(size_t)offset) {
+ if (ms->fpos < (size_t)(-offset)) {There was a problem hiding this comment.
Thanks, but my comment was referring to both cases. I suppose the fix is ok to avoid the undefined behavior, though it's probably not the optimal fix either.
There was a problem hiding this comment.
Thank you. Adding the following code in the first part of the php_stream_memory_seek function, what do you think?
if (offset == ZEND_LONG_MIN) {
zend_argument_value_error(2, "must be greater than " ZEND_LONG_FMT, ZEND_LONG_MIN);
return FAILURE;
}
#20964