Why does it have to cast to struct in_addr in this code?

Question

TCPStream::TCPStream(int sd, struct sockaddr_in* address) : msd(sd) 
{
    char ip[50];
    inet_ntop(PF_INET, (struct in_addr*)&(address->sin_addr.s_addr), ip, sizeof(ip)-1);
    m_peerIP = ip;
    m_peerPort = ntohs(address->sin_port);
}

Why does it have to cast to struct in_addr in this code?

What does '50' mean in this code?

[The Definitive C++ Book Guide and List](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) — PaulMcKenzie, Dec 11 '17 at 08:44
*"What does '50' mean in this code?"* - Same thing it means in every day life. How much of something there is. — StoryTeller - Unslander Monica, Dec 11 '17 at 08:55
"*cast to "`struct in_addr`" the code casts to a *pointer* to `struct in_addr`, namely to a `struct in_addr *`. — alk, Dec 11 '17 at 09:07
`50` means the programmer doesnt really know and is guessing a number that hopefully won't cause a buffer overflow — M.M, Dec 11 '17 at 09:10

Some programmer dude · Accepted Answer · 2017-12-12T05:56:19.670

4

You don't need a cast to convert to void*, which is what the second argument is.

You do however have a few other problems with that code.

Lets start with the first argument to the inet_ntop call: You use PF_INET. The prefix PF stands for Protocol Family. Since you're getting information about an address you should use the AF_INET symbolic constant instead, where AF stands for Address Family.

Another problem is that (unnecessary) cast you make. If you follow the structures, the sockaddr_in structure is defined in the <netinet/in.h> header file. The member sin_addr is of type in_addr. And in_addr have a single member s_addr which is of type in_addr_t (which is equivalent to uint32_t). That is, &(address->sin_addr.s_addr) is a pointer to an unsigned 32-bit integer type. Not struct in_addr* (that would be &address->sin_addr).

If we put all this together, the correct call should be

inet_ntop(AF_INET, &address->sin_addr.s_addr, ip, sizeof ip);

Now as for the size 50 for the array. That's a little to big, since an IPv4 address can be at most 16 characters (including the string terminator). This also happens to be the value of the macro INET_ADDRSTRLEN.

So the array could be defined as

char ip[INET_ADDRSTRLEN];

edited Dec 12 '17 at 05:56

answered Dec 11 '17 at 09:07

Some programmer dude

400,186
35
402
621

The *correct* call would be `inet_ntop(AF_INET, &(address->sin_addr), ip, sizeof ip);` instead. Apply the `&` operator to the `sin_addr` member, not to the `s_addr` member (which *happens* to be at the same address as `sin_addr` in this case, but it is not good to rely on this). `inet_ntop()` expects an `in_addr*`, so give it a pointer to an actual `in_addr` – Remy Lebeau Dec 11 '17 at 22:18
@RemyLebeau Weird, [the POSIX reference](http://pubs.opengroup.org/onlinepubs/9699919799/functions/inet_ntop.html) doesn't say anything about `struct in_addr`, only that "[t]he *src* argument points to a buffer holding an IPv4 address if the *af* argument is `AF_INET`" – Some programmer dude Dec 12 '17 at 05:57
[Linux](http://man7.org/linux/man-pages/man3/inet_ntop.3.html) and [Windows](https://msdn.microsoft.com/en-us/library/windows/desktop/cc805843.aspx) docs for `inet_ntop()` specifically state that `in_addr` is required – Remy Lebeau Dec 12 '17 at 06:18

score 1 · Answer 2 · answered Dec 11 '17 at 22:16

The address->sin_addr field is an in_addr struct, which has a single data field s_addr that is a uint32_t. So, the address of the s_addr field happens to be the same address as its containing in_addr. And while this kind of casting is "safe" to do in this case, it is also wrong to do.

Per the Linux documentation for inet_ntop():

AF_INET
        `src` points to a `struct in_addr` (in network byte order) which
        is converted to an IPv4 network address in the dotted-decimal
        format, `"ddd.ddd.ddd.ddd"`.  The buffer `dst` must be at least
        `INET_ADDRSTRLEN` bytes long.

As you can see, inet_ntop() expects an in_addr* pointer for an IPv4 address. The address->sin_addr field is an actual in_addr, so you should be passing the address of address->sin_addr to inet_ntop(), not the address of address->sin_addr.s_addr.

An IPv4 address takes up only 15 characters at most, plus a null terminator. So 50 is way overkill for your ip buffer size. 16 (what INET_ADDRSTRLEN is defined to be) will suffice.

The correct code should look more like this:

TCPStream::TCPStream(int sd, struct sockaddr_in* address) : msd(sd) 
{
    char ip[INET_ADDRSTRLEN];
    inet_ntop(AF_INET, &(address->sin_addr), ip, sizeof(ip));
    m_peerIP = ip;
    m_peerPort = ntohs(address->sin_port);    
}

Why does it have to cast to struct in_addr in this code?

2 Answers2