Add uv_device_t as stream on windows and linux, handle device I/O asyncly.#1580
Add uv_device_t as stream on windows and linux, handle device I/O asyncly.#1580zhaozg wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
My experience with windows has been that you get called back even if the write is synchronous so you can safely ignore "returned immediately" scenarios.
There was a problem hiding this comment.
It could be, if SetFileCompletionNotificationModes had been used.
But yes by default IOCP reports all succesful operations, even if they completed synchronously. So that looks like it needs to be fixed here.
|
In general it looks like it will work for my use case, I'm still (of course) favorable to my own patch, in this case because it's fewer lines of code, but if this is code which I do not maintain, I'm happy with that so +1 in theory. If you feel like patching https://github.com/cjdelisle/cjdns/blob/master/interface/tuntap/windows/TAPInterface.c to work with your version then I'll really love you :) |
|
Before getting into reviewing any code, I'd like to hear some general thoughts from @indutny, @bnoordhuis and @piscisaureus. The approach taken here looks clean and nice to me, if that's enough for people wanting to use serial devices (for example) not to (ab)use uv_fs_open, I'm all in :-) |
There was a problem hiding this comment.
This define device private field, for device handle, and read data buffer.
There was a problem hiding this comment.
I see you no longer malloc your own buffer, good! Can you please rename this to read_buffer?
|
Thanks @zhaozg! I did an initial review of the code (I didn't cover it all yet though). |
There was a problem hiding this comment.
This one is inconsistent with the other _open functions. Maybe we should remove it and open the device in _init? That's what uv_tty_init does, for example.
|
Without having really followed this conversation except to periodically sweep the mess of mail out of my inbox, I thought perhaps you would enjoy reading parkinson's law :) |
|
@cjdelisle :-) |
There was a problem hiding this comment.
Return -EINVAL instead of asserting.
|
@saghul I should make a new RP to libuv/libuv? |
|
If you don't mind, that would be great!
|
|
Close this RP, because replaced with libuv/libuv#19. |
I'm a multi-language programmer, but not family with english, so maybe trouble with options. libuv is a good i/o framework, and easy to extent, I hope persist on it to do my work.
When I want to do r/w device on windows, I try filesystem api, it works for device handle, but not fit my needs. I need a real stream, I try wrap handle as pipe, and fail to work. When I study TTY and TCP, I realize that it not hard to implication a device type stream, because just file operation with overlapped, even simpler than other stream types.
When I finish my job on windows, i realize libUV is born for linux, simple to add a device type, so i did it.
There are only three type related api.
uv_device_init will init and create a uv_deive_t, path should be '/dev/??' on linux or ".\Global???" on windows.
flags should be O_RDONLY,O_WRONLY or O_RDWR.
uv_device_ioctl will set or get device paramater, arg is relative with OS platform, return >=0 for success, detail please see header file.
because not test widely, so maybe have bugs, please attention.
Happy for do something for libUV.