Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add uv_fs_read() $offset param allowing seeking on open handles #60

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

rdlowrey
Copy link
Contributor

This PR modifies uv_fs_read() with a new long $offset parameter. This allows the same file handle to be reused for multiple reads. Previously the only way to access a section of a file that had already been read was to open a new handle.

Old:

uv_fs_read(resource $loop, zval $fd, long $length, callable $callback)

New:

uv_fs_read(resource $loop, zval $fd, long $offset, long $length, callable $callback)

This change represents a minor BC break for existing code using uv_fs_read().

This change modifies the uv_fs_read() function signature slightly by adding
a new $offset parameter. This allows the same file handle to be reused for
multiple reads. Previously the only way to access a section of a file that
had already been read was to open a new handle.

Old:
uv_fs_read(resoruce $loop, zval $fd, long $length, callable $callback)

New:
uv_fs_read(resoruce $loop, zval $fd, long $offest, long $length, callable $callback)

This change represents a minor BC break for existing code using uv_fs_read().
@rdlowrey rdlowrey changed the title Add $offset param to uv_fs_read() to allow seeking on open handles Add uv_fs_read() $offset param allowing seeking on open handles Nov 11, 2014
@steverhoades
Copy link

+1!

@steverhoades
Copy link

LOL! Didn't notice that but your right, I agree - consistency in that regard makes sense.

steverhoades added a commit to steverhoades/php-uv that referenced this pull request Nov 23, 2014
@steverhoades
Copy link

Hope you dont mind but I merged this PR into the libuv 1.0 WIP.

@rdlowrey
Copy link
Contributor Author

Yeah that's no problem @steverhoades ...

Pinging @chobie to see if we can get this PR merged. I know he's busy ... I'm forced to ignore PRs for long periods sometimes because of other work too :)

@joshdifabio
Copy link

@chobie How do you feel about merging this?

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.

3 participants