API Bug of the Week: getsockname().

A “non-blocking” IPv6 connect() call was in fact, blocking.  Tracking that down made me realize the IPv6 address was mostly random garbage, which was caused by this function:

bool get_fd_addr(int fd, struct protocol_net_address *addr)
{
   union {
      struct sockaddr sa;
      struct sockaddr_in in;
      struct sockaddr_in6 in6;
   } u;
   socklen_t len = sizeof(len);
   if (getsockname(fd, &u.sa, &len) != 0)
      return false;
   ...
}

The bug: “sizeof(len)” should be “sizeof(u)”.  But when presented with a too-short length, getsockname() truncates, and otherwise “succeeds”; you have to check the resulting len value to see what you should have passed.

Obviously an error return would be better here, but the writable len arg is pretty useless: I don’t know of any callers who check the length return and do anything useful with it.  Provide getsocklen() for those who do care, and have getsockname() take a size_t as its third arg.

Oh, and the blocking?  That was because I was calling “fcntl(fd, F_SETFD, …)” instead of “F_SETFL”!

3 replies on “API Bug of the Week: getsockname().”

  1. The traditional reason for making a deliberately incomplete getsockname() call is when you have a socket of an unknown type; you call getsockname() with a basic struct sockaddr, then inspect sockaddr.sa_family afterwards (and possibly re-call it with the right struct). This doesn’t strictly require a writable len argument but it does require that the ‘incomplete’ sockaddr be filled in as far as possible even on short lengths (or at least if the length is just the basic struct sockaddr).

  2. How about cleaning up C. Clearly defined primative types only int16,int32 etc. Structures only in heap memory. You could really clean it up mainly by “taking away” and only adding one or 2 small things like namespaces. I would use C more if I didn’t have to keep guessing what someone else meant by int.

Comments are closed.