1

I am trying to create a void mksockaddr(int af, int proto, char addr[], struct sockaddr* dst) that creates a sockaddr structure, here's what I've done:

void sockaddr(int af, int port, char addr[], struct sockaddr* dst) {
  if (af == AF_INET) {
    struct sockaddr_in s;
    s.sin_family = af;
    s.sin_port = htons(port);
    inet_pton(af, addr, &s.sin_addr);
    memcpy(dst, &s, sizeof(s));
  } else {
    struct sockaddr_in6 s;
    s.sin6_family = af;
    s.sin6_port = htons(port);
    s.sin6_flowinfo = 0;
    inet_pton(af, addr, &s.sin6_addr);
    memcpy(dst, &s, sizeof(s));
  }
}

This seems to be no problem with AF_INET (IPv4), I can bind() without any problem, but when I try to use AF_INET6, bind() give me Invalid argument.

Here's the code I use to bind():

int sock_fd = socket(AF_INET6, SOCK_RAW, proto);
struct sockaddr sin;
sockaddr(AF_INET6, proto, src, &sin);
if(bind(sock_fd, &sin, sizeof(sin)) < 0) {
  fprintf(stderr, "[ERR] can't bind socket: %s\n", strerror(errno));
  exit(1);
} // got Invalid argument

However, I can bind() just fine if I construct a sockaddr_in6 myself:

struct sockaddr_in6 sin;
sin.sin6_port = htons(proto);
sin.sin6_family = AF_INET6;
inet_pton(AF_INET6, src, &sin.sin6_addr);
if(bind(sock_fd, (struct sockaddr*) &sin, sizeof(sin)) < 0) {
  fprintf(stderr, "[ERR] can't bind socket.\n");
  exit(1);
} // work just fine

So I cast the sockaddr created by the function back to sockaddr_in6, and I can see that all the fields are same except sin6_scope_id. To my understanding, sin6_scope_id does not matter unless I'm dealing with a link-local IPv6 address.

Am I missing anything here?

  • See [Adding support for IPv6 in IPv4 client/server apps - sin6_flowinfo and sin6_scope_id fields?](https://stackoverflow.com/questions/8256671/adding-support-for-ipv6-in-ipv4-client-server-apps-sin6-flowinfo-and-sin6-scop). – Richard Chambers Jan 18 '18 at 19:28
  • 2
    if your "works just fine" code you're doing `htons(proto)`,, I assume that's supposed to be `htons(port)`,, but I don't know why that would make or break it. – yano Jan 18 '18 at 19:31
  • 1
    How are you *using* this function? In particular, what are you passing to the `dst` argument? Please edit your question to include an example call for each supported case. – John Bollinger Jan 18 '18 at 19:38
  • @JohnBollinger See the 2nd code block, is called using `sockaddr(AF_INET6, proto, src, &sin);`. – Morichika Maho Jan 18 '18 at 19:54
  • 2
    You should rather use `getaddrinfo` which handles the v4/v6 cases for you. – Aif Jan 18 '18 at 20:26

1 Answers1

1

From a C perspective, for your code to be certain to work as intended, the caller must pass a valid pointer to the correct structure type in the dst argument. Your example does not do this. Instead, it declares a struct sockaddr, and passes a pointer to that. Type struct sockaddr itself is never meant to be used as the type of an actual object, and it is not large enough for all possible address types. In particular, it is not large enough for an IPv6 address.

On the other hand, POSIX plays a bit more fast and loose than standard C requires for conforming programs. This is especially evident with socket addresses. It defines a type struct sockaddr_storage to serve exactly your purpose: it is large enough and has appropriate alignment to hold the data of any supported socket address type. The docs specifically mention its use in generically supporting both IPv4 and IPv6. POSIX also sanctions casting among different socket address pointer types, although this leads to violations of C's struct aliasing rule.

Thus, I would rewrite your function to use struct sockaddr_storage explicitly, and I would furthermore simplify my code via appropriate casts. Moreover, I would have my function tell me the usable size of the address structure, which encompasses only that portion that is initialized:

void populate_sockaddr(int af, int port, char addr[],
        struct sockaddr_storage *dst, socklent_t *addrlen) {

    if (af == AF_INET) {
        struct sockaddr_in *dst_in4 = (struct sockaddr_in *) dst;

        *addrlen = sizeof(*dst_in4);
        memset(dst_in4, 0, *addrlen);
        dst_in4->sin_family = af;
        dst_in4->sin_port = htons(port);
        inet_pton(af, addr, &dst_in4->sin_addr);

    } else if (af == AF_INET6) {
        struct sockaddr_in6 *dst_in6 = (struct sockaddr_in6 *) dst;

        *addrlen = sizeof(*dst_in6);
        memset(dst_in6, 0, *addrlen);
        dst_in6->sin6_family = af;
        dst_in6->sin6_port = htons(port);
        // unnecessary because of the memset(): dst_in6->sin6_flowinfo = 0;
        inet_pton(af, addr, &dst_in6->sin6_addr);
    } // else ...
}

You would then use it like so:

struct sockaddr_strorage addr;
socklen_t addrlen;

populate_sockaddr(af, port, src, &addr, &addrlen);
if (bind(sock_fd, (struct sockaddr *) &addr, addrlen) < 0) {
    // ...
}

Note that the cast of &addr to type struct sockaddr * is utterly routine.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
John Bollinger
  • 160,171
  • 8
  • 81
  • 157
  • Might want to change `Your example does not do not do this.` – sjsam Jan 18 '18 at 20:29
  • @JohnBollinger "*Your example does not do not do this*" is bad grammar, that is what sjsam is referring to. It should say "*Your example does not do this*" instead. I made the edit (and a few others). – Remy Lebeau Jan 20 '18 at 07:36
  • 2
    @JohnBollinger your solution is replicating what `getaddrinfo()` already does when passed the `AI_NUMERICHOST` flag. Best to use an existing standard function than invent a custom one. – Remy Lebeau Jan 20 '18 at 07:39