Skip to content

Conversation

@cmb69
Copy link
Member

@cmb69 cmb69 commented Aug 10, 2021

When flushing the stream filters actually causes data to be written to
the stream, we need to update its position, because that is not done by
the streams' write methods.

When flushing the stream filters actually causes data to be written to
the stream, we need to update its position, because that is not done by
the streams' write methods.
@nikic
Copy link
Member

nikic commented Aug 10, 2021

Something I'm a bit confused about is this code in write_buffer:

php-src/main/streams/streams.c

Lines 1148 to 1152 in 1c675b9

/* Only screw with the buffer if we can seek, otherwise we lose data
* buffered from fifos and sockets */
if (stream->ops->seek && (stream->flags & PHP_STREAM_FLAG_NO_SEEK) == 0) {
stream->position += justwrote;
}

It only changes the position for seekable streams, though I don't really understand the rationale given in the comment.

@nikic
Copy link
Member

nikic commented Aug 10, 2021

Looks like the code originally looked like this: f98f27f Possibly the condition around the position update is just a leftover and it should be updated unconditionally?

@cmb69
Copy link
Member Author

cmb69 commented Aug 10, 2021

I think you're right; 088e269#diff-34c102f51eb0af2504e7d1c1f588a83c6555ae92d2f83602314ea45fc94621b4 looks fishy; not updating the position for non-seekable streams seems to be a bug. I'm going to check that.

@cmb69
Copy link
Member Author

cmb69 commented Aug 10, 2021

See PR #7356.

@cmb69 cmb69 closed this in 40b31fc Aug 10, 2021
@cmb69 cmb69 deleted the cmb/81302 branch August 10, 2021 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants