Handling of file descriptor in win64 broken.

[rcane]

In the api a file descriptor (e.g. in poller::add()) is a simple int. But under Windows such a descriptor is actually of type SOCKET. This type is a typedef to UINT_PTR (see WinSock2.h). And here is the problem: Under 32bit UINT_PTR is an unsigned int, but under x64 it is an unsigned __int64. This means that the value of a file descriptor under x64 cannot not fit into an int. The compiler only warns about this but happily cuts off half of the value by casting the uint64 into int. In case of the poller this could have some unexpected side effects. Two distinct descriptors could be seen as the same because they only differ in the upper 32 bit. Worse than that is that the poller could do the wrong thing altogether because the 'broken' descriptor value is passed to zmq_poll().

A solution to this would be to not use int as the file descriptor but a type that is declared depending on the platform like this:

#ifdef _WIN32
    typedef SOCKET descriptor_t;
#else
    typedef int descriptor_t;
#endif

void poller::add(descriptor_t const descriptor, short const event /* = POLL_IN */);

Of course this would break the public interface. But seeing that it only breaks for windows and it is already broken there, I see no other choice.

[xaqq]

Of course this would break the public interface. But seeing that it only breaks for windows and it is already broken there, I see no other choice.

We could do that indeed. However, do you know how zmq_pool() work on win64? What type does it expect when receiving a file descriptor on window64? Maybe we could reuse that type in the zmqpp's interface (assuming this type work on all platform)?

What do you think?

[rcane]

zmq_pool() uses struct zmq_pollitem_t (see zmq.h):

typedef struct zmq_pollitem_t
{
    void *socket;
#if defined _WIN32
    SOCKET fd;
#else
    int fd;
#endif
    short events;
    short revents;
} zmq_pollitem_t;

This struct is in fact the place where SOCKET comes into play. So to answer your question zmq always uses the SOCKET type under windows in its api but somewhat hides that in zmq_pollitem_t. This is what gave me the idea to suggest the same thing for zmqpp. Another way of fixing this would be to remove the methods taking just the descriptor and forcing people to use the ones taking a zmq_pollitem_t. But we would lose the convenience there and break the api for all platforms.

To summarize: making the change I suggested would 'repair' win64 and break the api for both win32 and win64 and do nothing to the other platforms.

[xaqq]

Yes using zmq_pollitem_t would be too inconvenient for no real benefit here. Your change is the way to go then.

I don't grasp the whole problem since I am not familiar with the way file descriptor are represented on windows. Is there any case where using a simple int make sense and where losing this feature under window would be problematic? What I mean is there any reason to keep providing an void poller::add(int const descriptor, short const event /* = POLL_IN */); method while having a SOCKET version?

[rcane]

The whole thing with file descriptors in this context is quite confusing because under windows there aren't any. ;-) This file descriptor concept comes from POSIX where file handles are used for things other than actual physical files on a disk. In Windows a file is really just a file. That is why the socket api in windows only uses 'sockets' and instead of using a 'file descriptor' they invented the SOCKET type.

So I don't see any reason to keep the int version of poller::add() for the windows platform because a client will always have to supply a SOCKET to use this method.

The documentation of add() should also be changed to not talk about a 'file descriptor' directly but rather use the phrase zmq_poll() uses ...standard socket specified by the file descriptor.... This makes it much clearer that this method expects a raw socket and has nothing to do with actual files.

[xaqq]

Thank you for clarifying the subject for me.

So I don't see any reason to keep the int version of poller::add() for the windows platform because a client will always have to supply a SOCKET to use this method.

Ok.

The documentation of add() should also be changed to not talk about a 'file descriptor' directly but rather use the phrase zmq_poll() uses ...standard socket specified by the file descriptor.... This makes it much clearer that this method expects a raw socket and has nothing to do with actual files.

Fair enough.

Please submit a PR making those changes then :)