unix, windows: add uv_fileno

[saghul]

Returns the platform specific file descriptor for handles that are
backed by one. The datatype is abstracted as uv_os_fd_t, which maps to
int on Unices and HANDLE on Windows.

Users can use this function to set specific socket options, for example,
in a non portable way.

This function is essentially a shotgun, you better be careful with
whatever you do with it, don't blame me if you used it to get the fd of
a stream, close it yourself and expect things to Just Work.

[saghul]

This patch is unifinished (Windows part and tests are missing), but I wanted to start a discussion about it. I can't think of a way which would please everyone while keeping our fds hidden, so I think it might not be that bad to expose them for those with legit use cases such as setting some socket options here and there, we can't possibly map all of them.

About the Windows side, HANDLE is just a void*, which means we wouldn't be able to cast it to int because of 64bit systems. However, reading around seems like only the lower 32 bits are used because a process can only have 2^24 handles in that handle table. Anyway, I went for the typedef, since this is a platform dependent thing and feels more future-proof.

Thoughts?

Calling the cavalry! @indutny, @bnoordhuis, @piscisaureus

[txdv]

IMO this is a good idea.

Let the user do whatever he wants.

[txdv]

This is such an easy fix, why didn't we have it earlier?

[saghul]

Just like with scotch, some things take time to become better, I guess :-P

[bnoordhuis]

Maybe UV_EXTERN int uv_file(const uv_handle_t* handle, uv_os_fd_t* fd);? Libuv can't really return a meaningful error now.

[saghul]

That crossed my mind, but in the end I thought that I'd just return -1 when there is no fd. We could give EINVAL if you pass an unsupported handle type, I guess.

[bnoordhuis]

But how are you going to do that on Windows? HANDLE is a typedef for void*.

[saghul]

I guess the same way Windows does for INVALID_SOCKET :-P On a more serious note, I do like your proposal, I'll change this.

[saghul]

FWIW, Windows defines INVALID_SOCKET as SOCKET(~0).

[saghul]

Updated the API and Unix implementation as per Ben's suggestions. It returns EINVAL if the handle is not a stream, poll or udp and EBADF if the fd is -1.

[bnoordhuis]

It's kind of bad style to return an error code but still mutate the out parameter, IMO.

[saghul]

True that, will fix.

[saghul]

Added tests and Windows implementation. The TTY test fails, I need to open $conin it seems, like the TTY test does, will do.

[bnoordhuis]

Can't you filter on UV__HANDLE_CLOSING in uv_fileno()?

[saghul]

I'm not sure what guarantees we make about it. The handle (libuv wise) is closing, but the actual Windows HANDLE might need to stick around for a bit more.

[saghul]

Tests pass on Windows now.

[saghul]

@bnoordhuis thanks, const-ified the handle now.

[saghul]

Any further comments on this?

[txdv]

Why do we set it to invalid?

[saghul]

Why do we set it to invalid?

Because otherwise you get it with uv_fileno after you have closed it so it might belong to another handle that reused it. This wasn't a problem before, because it was not exposed, but now that people get access to file descriptors, we cannot give them stalled ones.

Another possibility might be to return EBADF if uv_is_closing(handle). That might actually simplify things a bit...

[saghul]

I'll give that a shot (I think that's what @bnoordhuis said before, but I didn't quite get it until now). I'd still leave the commit that sets handles to invalid, as a defense mechanism.

[saghul]

Updated with uv_is_closing check, now we no longer need any ifdef-ery on the tests \o/.

[bnoordhuis]

LGTM. And yes, that was what I meant. :-)

[saghul]

Thanks folks, landed! Load your shotguns.