[zeromq-dev] Binding to TCP port 0

[Pierre Ynard]

Hi,

> I have knocked up a relatively simple patch to implement this on IPC
> and TCP transports, would appreciate comments as I'm pretty sure
> there's some dodgy stuff in there! I've made a pull req, though
> mainly for comment (please don't pull it chuck/mikko/pieter!):
> https://github.com/zeromq/libzmq/pull/238

I like your patch, and I'm looking forward to try it!

> At the moment is only looks for the wildcards on bind, connect looked
> a bit trickier, but I'm sure it would just be finding the right place.
> Inproc should be fairly straightforward, but I don't think there's as
> much of a use case there.

I'm confused, I don't really understand the meaning of connecting to a
wildcard.

A few comments follow:

> +    // Allow wildcard file
> +    if(*addr_ == '*') {
> +        addr_ = tempnam(NULL, NULL);
> +    }

Hmm this brings a race condition, between the moment when the
availability of the file name is checked, and the moment the socket is
created. I don't really know how to prevent this :/

> +        // Last socket endpoint URI
> +        unsigned char last_endpoint [256];
> +        size_t last_endpoint_size;

Wouldn't it be better to use ZMQ_ENDPOINT_MAX here?

> +    uint16_t port;
> +    if (port_str[0] == '*') {
> +        // Resolve wildcard to 0 to allow autoselection of port
> +        port = 0;
> +    } else {
> +        //  Parse the port number (0 is not a valid port).
> +        port = (uint16_t) atoi (port_str.c_str());

I think it would be better and simpler to accept both * and 0

> +    rc = getsockname (s, &sa, &sl);   
> +    if (rc == 0) {
> +        char host[INET6_ADDRSTRLEN];
> +        int port;
> +        
> +        if ( sa.sa_family == AF_INET ) {
> +            inet_ntop(AF_INET, &(((struct sockaddr_in *)&sa)->sin_addr), host, INET6_ADDRSTRLEN);
> +            port = ntohs( ((struct sockaddr_in *)&sa)->sin_port);
> +        } else {
> +            inet_ntop(AF_INET6, &(((struct sockaddr_in6 *)&sa)->sin6_addr), host, INET6_ADDRSTRLEN);
> +            port = ntohs( ((struct sockaddr_in6 *)&sa)->sin6_port);
> +        }
> +        
> +        // Store the address for retrieval by users using wildcards
> +        bound_addr_len = sprintf(bound_addr, "tcp://%s:%d", host, port);

You probably want to enclose the address within [ ] if it's IPv6.

Also, is it really necessary to use a bound_addr buffer? It seems to me
that in get_address() you could directly call getsockname() on the
socket file descriptor, and build the address string directly in the
supplied buffer. Normally this could work with IPC sockets too.

Regards,

-- 
Pierre Ynard
"Une âme dans un corps, c'est comme un dessin sur une feuille de papier."