joyent/libuv

Please expose bufs in uv_fs_t's result for uv_fs_read operations.

Opened this issue · 3 comments

In my bindings, uv_fs_read is the only place in all of libuv where I have to externally track the read buffer to get the result. In others like uv_read, the callback gives me a uv_buf_t which contains the buffer I allocated for the read (in uv_alloc_cb). This makes is easy to both get the result and free my data when I'm done.

It would be awesome and save me a pointer in my reqs if uv_fs_read did the same thing and gave me back the buffer somehow in the result. It appears the req->ptr member isn't used for anything in this fs operation but I'm not entirely sure how you would fit both the bufs array and the length of the array.

For my uses, the array is always length 1 so that's not a problem.

Also it appears that there are already private members containing the bufs and the number of bufs. If these were simply added to the public API, that would be enough.

You can embed your uv_fs_t struct into another one and use container_of to grab a pointer to it on the callback.

This is the same design principle taken for uv_write for example. I don't recall what the reasons behind this were, maybe @bnoordhuis can clarify, but IIRC it has been like this from the start and the use of structure embedding is recommended.

Now, between you and I, those fields are not going away on 1.x ;-)

I don't recall what the reasons behind this were, maybe @bnoordhuis can clarify, but IIRC it has been like this from the start and the use of structure embedding is recommended.

Embedding allows for compact structures that can be freed in one go. It eliminates the possibility of dangling pointers. Embedding is generally also a bit faster than pointer chasing, although that's mostly irrelevant in libuv.

Having public API accessors would be a good thing. It lets us move things around in the future without having to worry about breakage and it's arguably all around cleaner than letting people grub around in your private parts, amirite?