Gluster users - Sep 2009 - 2.0.6/transport/socket/src/name.c af_inet_bind_to_port_lt_ceiling usage

Hello programmers,

here you can see some code from above source that you really do not want if
you like defensive programming:


int32_t
client_bind (transport_t *this,
             struct sockaddr *sockaddr, 
             socklen_t *sockaddr_len,
             int sock)
{
        int ret = 0;

        *sockaddr_len = sizeof (struct sockaddr_in6);
        switch (sockaddr->sa_family)
        {
        case AF_INET_SDP:
        case AF_INET:
                *sockaddr_len = sizeof (struct sockaddr_in);

        case AF_INET6:
                ret = af_inet_bind_to_port_lt_ceiling (sock, sockaddr,
                                                       *sockaddr_len,
CLIENT_PORT_CEILING);
                if (ret == -1) {
                        gf_log (this->xl->name, GF_LOG_ERROR,
                                "cannot bind inet socket (%d) to port less
than %d (%s)",
                                sock, CLIENT_PORT_CEILING, strerror (errno));
                        ret = 0;
                }
                break;

        case AF_UNIX:
                *sockaddr_len = sizeof (struct sockaddr_un);
                ret = af_unix_client_bind (this, (struct sockaddr *)sockaddr,
                                           *sockaddr_len, sock);
                break;
 
        default:
                gf_log (this->xl->name, GF_LOG_ERROR,
                        "unknown address family %d",
sockaddr->sa_family);
                ret = -1;
                break;
        }
 
        return ret;
}

If you look closely you find that the error case of
af_inet_bind_to_port_lt_ceiling
returning -1 is ending up as ret=0, although all other error cases of this
function
return -1. Which means this function returns 0 or -1 in case of error and 0 in
case
of nonerror. No chance to make anything useful of the return value for a caller.

-- 
Regards,
Stephan

> here you can see some code from above source that you really do not want if
> you like defensive programming:
>
>
> int32_t
> client_bind (transport_t *this,
> ? ? ? ? ? ? struct sockaddr *sockaddr,
> ? ? ? ? ? ? socklen_t *sockaddr_len,
> ? ? ? ? ? ? int sock)
> {
> ? ? ? ?int ret = 0;
>
> ? ? ? ?*sockaddr_len = sizeof (struct sockaddr_in6);
> ? ? ? ?switch (sockaddr->sa_family)
> ? ? ? ?{
> ? ? ? ?case AF_INET_SDP:
> ? ? ? ?case AF_INET:
> ? ? ? ? ? ? ? ?*sockaddr_len = sizeof (struct sockaddr_in);
>
> ? ? ? ?case AF_INET6:
> ? ? ? ? ? ? ? ?ret = af_inet_bind_to_port_lt_ceiling (sock, sockaddr,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? *sockaddr_len,
CLIENT_PORT_CEILING);
> ? ? ? ? ? ? ? ?if (ret == -1) {
> ? ? ? ? ? ? ? ? ? ? ? ?gf_log (this->xl->name, GF_LOG_ERROR,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"cannot bind inet socket (%d) to port
less than %d (%s)",
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sock, CLIENT_PORT_CEILING, strerror
(errno));
> ? ? ? ? ? ? ? ? ? ? ? ?ret = 0;
> ? ? ? ? ? ? ? ?}
> ? ? ? ? ? ? ? ?break;
>
> ? ? ? ?case AF_UNIX:
> ? ? ? ? ? ? ? ?*sockaddr_len = sizeof (struct sockaddr_un);
> ? ? ? ? ? ? ? ?ret = af_unix_client_bind (this, (struct sockaddr
*)sockaddr,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? *sockaddr_len, sock);
> ? ? ? ? ? ? ? ?break;
>
> ? ? ? ?default:
> ? ? ? ? ? ? ? ?gf_log (this->xl->name, GF_LOG_ERROR,
> ? ? ? ? ? ? ? ? ? ? ? ?"unknown address family %d",
sockaddr->sa_family);
> ? ? ? ? ? ? ? ?ret = -1;
> ? ? ? ? ? ? ? ?break;
> ? ? ? ?}
>
> ? ? ? ?return ret;
> }
>
> If you look closely you find that the error case of
af_inet_bind_to_port_lt_ceiling
> returning -1 is ending up as ret=0, although all other error cases of this
function
> return -1. Which means this function returns 0 or -1 in case of error and 0
in case
> of nonerror. No chance to make anything useful of the return value for a
caller.

Unlike what you think, the code has the intended behavior. It makes
the best effort to bind to a port less than the ceiling value. If it
cannot bind to port less than 1024, it proceeds anyways with the hope
that the server is not doing a host based authentication check.

BTW, such mails are best posted to gluster-devel.

Avati