2

I have a sockaddr_storage object and I need to fill it with values provided by the user. Note that user can provide either of AF_INET or AF_INET6 as domain for filling up the struct.


void fill(sockaddr_storage &addrStruct, int domain,
      const char addr[], const int port)
{
    std::memset(&addrStruct, 0, sizeof(addrStruct));

    switch(domain) {

        case AF_INET: addrStruct.sin_family = AF_INET;
                      addrStruct.sin_port= htons(port);

                      inet_pton(AF_INET, addr, addrStruct.sin_addr);

        case AF_INET6: ....
        ....
        ....
        default: ....
    }
}

Pretty sure this doesn't works since addrStruct is of type struct sockaddr_storage and these members are present in struct sockaddr_in. I also tried static_cast<sockaddr_in>(addrStruct).sin_port and similar but that again doesn't works. So how should I be filling up this struct so that it holds valid values while respecting alignment of casted structs.

Kolay.Ne
  • 1,345
  • 1
  • 8
  • 23
Abhinav Gauniyal
  • 7,034
  • 7
  • 50
  • 93
  • Use `getaddrinfo` instead of what you are doing, and don't worry about it. – zwol Feb 11 '17 at 14:23
  • @zwol getaddrinfo might increase complexity of my function since I'm already returning error(or success) values of `inet_pton` calls, I'll have to take another call into consideration. Also `getaddrinfo` tends to return multiple addresses on occasion, I don't remember the scenario well, but that would again increase complexity.. – Abhinav Gauniyal Feb 11 '17 at 14:26
  • Try it. You will find that the complexity goes _down_ significantly. – zwol Feb 11 '17 at 14:27

3 Answers3

4

You need to use the appropriate address structure first, and fill it out:

struct sockaddr_in sin;

sin.sin_family = AF_INET;
sin.sin_port= htons(port);

Then, after you're done, memcpy() it into the sockaddr_storage:

memcpy(reinterpret_cast<char *>(&addrStruct),
       reinterpret_cast<char *>(&sin), sizeof(sin));

Note that you should still zero out the full sockaddr_storage buffer beforehand, as you did.

Another way to do it is to define your own union of all the address structures, including sockaddr_storage, then initialize the appropriate union member.

Sam Varshavchik
  • 114,536
  • 5
  • 94
  • 148
  • yes I was thinking of defining a union with `sockaddr`, `sockaddr_in`, `sockaddr_in6` and `sockaddr_storage` but I'm afraid that might break some aliasing rules(?) as I've read somewhere on SO only.. – Abhinav Gauniyal Feb 11 '17 at 14:28
  • Copying with `memcpy` like this is safe, and so is a `union` as long as you always work with it through the union type. The answer you accepted, on the other hand, definitely _does_ break the aliasing rules. – zwol Feb 11 '17 at 15:31
  • @Sam Varshavchik why the `reinterpret_cast`, since `memcpy` usually takes `void *`. If it is because of aliasing issues, then why now `unsigned char *`? – Abhinav Gauniyal Feb 20 '17 at 08:44
2

It is almost always better to use getaddrinfo than the inet_* family of address conversion functions. It does all of the allocation and initialization of sockaddr objects for you—you don't have to mess with sockaddr_storage yourself. It handles IPv4 and IPv6 seamlessly, it handles "multihomed" hosts with more than one address, and it can (optionally) do DNS lookups.

To use getaddrinfo, you completely throw away the fill function that you showed. The code that uses fill probably looks something like this:

struct sockaddr_storage ss;
fill(&ss, AF_INET, "10.0.0.21", 25);
int sock = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
if (sock == -1) {
     perror("socket");
     return -1;
}
if (connect(sock, (struct sockaddr*)&ss, sizeof(struct sockaddr_in)) {
     perror("10.0.0.21:25");
     return -1;
}
/* use connected sock here */

You replace that with

struct addrinfo hints;
struct addrinfo *rp, *result;
memset(hints, 0, sizeof hints);
hints.ai_family = AF_UNSPEC;
hints.ai_flags = AI_ADDRCONFIG;
hints.ai_socktype = SOCK_STREAM;

int err = getaddrinfo("10.0.0.21", "25", &hints, &result);
if (err) {
    if (err == EAI_SYSTEM) {
        fprintf(stderr, "10.0.0.21:25: %s\n", strerror(errno));
    } else {
        fprintf(stderr, "10.0.0.21:25: %s\n", gai_strerror(err));
    }
    return 1;
}

int sock;
for (rp = result; rp; rp = rp->ai_next) {
    sock = socket(rp->ai_family, rp->ai_socktype,
                  rp->ai_protocol);
    if (sock == -1)
        continue;
    if (connect(sock, rp->ai_addr, rp->ai_addrlen))
        break;
    close(sock);
}
if (rp == 0) {
    perror("10.0.0.21:25");
    return -1;
}
freeaddrinfo(result);
/* use sock here */

That looks more complicated, but remember that it totally replaces the fill function you didn't know how to write, and also remember that it will seamlessly handle DNS for you. (If you have a concrete reason to want to process numeric addresses only, you can set flags in hints for that.)

For more detailed example usage see the manpage I linked to.

zwol
  • 135,547
  • 38
  • 252
  • 361
1

You have to do reinterpret cast of addrStruct to sockaddr_in. In your case the code will look as following:

    case AF_INET: 
    {
        struct sockaddr_in * tmp =
            reinterpret_cast<struct sockaddr_in *> (&addrStruct);
        tmp->sin_family = AF_INET;
        tmp->sin_port = htons(port);
        inet_pton(AF_INET, addr, tmp->sin_addr);
    }
    break;

But I recommend you to use getaddrinfo(3) instead your approach.

J. Doe
  • 429
  • 2
  • 12
  • 1
    This code violates the "strict aliasing" rules and has a decent chance of being broken by modern compilers. Sad but true. – zwol Feb 11 '17 at 15:30
  • @zwol why does it break "strict aliasing" rules? I know `sockaddr_storage` can hold both `sockaddr_in/6` structs, `sockaddr_storage` struct is also said to be aligned perfectly to both of them.. see - http://pubs.opengroup.org/onlinepubs/009696699/basedefs/sys/socket.h.html, https://msdn.microsoft.com/en-us/library/windows/desktop/ms740504(v=vs.85).aspx – Abhinav Gauniyal Feb 11 '17 at 15:40
  • @zwol it's clearly mentioned that - "The structure shall be aligned at an appropriate boundary so that pointers to it can be cast as pointers to protocol-specific address structures and used to access the fields of those structures without alignment problems" and "Such alignment enables protocol-specific socket address data structures to access fields within a SOCKADDR_STORAGE structure without alignment problems." – Abhinav Gauniyal Feb 11 '17 at 15:41
  • The strict aliasing rules don't care about any of that. `struct sockaddr_storage`, `struct sockaddr_in`, and `struct sockaddr_in6` are different types. Therefore, declaring a variable with one of those types, and then accessing it through a pointer to a different one, has undefined behavior. It's really that simple. – zwol Feb 11 '17 at 15:44
  • @AbhinavGauniyal The wording you quote from POSIX was indeed _intended_ to work around the strict aliasing rules, but compiler badger don't care. – zwol Feb 11 '17 at 15:44
  • @zwol thankyou and I'm going to open another question on SO for the same to understand it better. – Abhinav Gauniyal Feb 11 '17 at 15:47
  • @AbhinavGauniyal There are several existing discussions of this, e.g. https://stackoverflow.com/questions/1429645/how-to-cast-sockaddr-storage-and-avoid-breaking-strict-aliasing-rules https://stackoverflow.com/questions/37661031/why-does-posix-contradict-the-iso-c-standards https://stackoverflow.com/questions/39425409/is-it-legal-to-type-cast-pointers-of-different-struct-types-e-g-struct-sockadd – zwol Feb 11 '17 at 15:50
  • @zwol but there are these too - http://stackoverflow.com/questions/33319224/casting-between-sockaddr-and-sockaddr-in , http://stackoverflow.com/questions/20130415/casting-sockaddr-storage-as-sockaddr-in-for-inet-ntop , http://stackoverflow.com/questions/18609397/whats-the-difference-between-sockaddr-sockaddr-in-and-sockaddr-in6 – Abhinav Gauniyal Feb 11 '17 at 15:54
  • @zwol I really don't want to make a mistake here and play safe so I would like to get some more opinions and take the decision after them. rest assured, I'm not trolling, just after an elegant solution. – Abhinav Gauniyal Feb 11 '17 at 15:57
  • @AbhinavGauniyal Yeah, so unfortunately there is a _lot_ of misinformation surrounding this topic, both on this site and elsewhere. And with `sockaddr` specifically, the API was designed before C89, so they didn't take the aliasing rules into account and using it safely is unergonomic. Hey, another reason to use `getaddrinfo`! Notice the complete absence of casts in my sample code? – zwol Feb 11 '17 at 15:58
  • @AbhinavGauniyal: The aliasing rules of C89 say that a pointer to a structure containing a type may be used to access an object of that type, and nothing in those rules says that the object actually needs to be part of a structure. The Common Initial Sequence rule was useful, and the lack of any requirement regarding the structure type itself meant that compilers had no legitimate basis for refusing to honor it. So far as I can tell, it was only after C99 was published, claiming to be merely a "clarification" of the C89 rules, that anyone took seriously the notion that C89 didn't... – supercat Feb 17 '17 at 20:21
  • ...allow constructs like the sock_addr family of structures. – supercat Feb 17 '17 at 20:22
  • @supercat good to know, although I'm more concerned about the present state of network programming. Whereas Microsoft specially mentions 'it is allowed' in their documents as well as their own compiler has no notion of strict aliasing as far as I know, which is same as clang, so this puts gcc into the corner. I haven't really found out a better way to deal with this problem so I'm using a large number of overloaded methods for now. – Abhinav Gauniyal Feb 18 '17 at 07:27
  • 1
    @AbhinavGauniyal: Unless the author of a compiler knows how to exercise a level of judgment beyond the bare minimum required by a Standard, the only safe way to write code which needs to do anything remotely interesting with storage is `-fno-strict-aliasing` or equivalent. A compiler that offered a relatively conservative aliasing mode and a non-conforming aliasing mode could achieve better optimization with a program that used each mode when appropriate, than would be possible with a compiler that used the most aggressive aliasing allowed by the Standard. I would think that common sense... – supercat Feb 18 '17 at 18:39
  • 1
    ...would suggest that such an approach would be both easier and in pretty much every way better than trying to eke the last bit of allowable optimization from a standard whose authors gave no consideration to the needs of 21st-century pipeline optimizations, but for whatever reason compiler writers seem to have no interest in such an approach. – supercat Feb 18 '17 at 18:41