Converting IP addresses from char * to uin32_t

Question

I was inspired by Conversion of IP address to integer

My code looks like this:

    uint32_r parseIPV4string(char * ipAddress){
    char ipbytes[4];
    sscanf(ipAddress, "%uhh.%uhh.%uhh.%uhh", &ipbytes[3], &ipbytes[2], &ipbytes[1], &ipbytes[0]);
    return ipbytes[0] | ipbytes[1] << 8 | ipbytes[2] << 16 | ipbytes[3] << 24;
}

Virtually an exact copy, my problem is however, that my ip addresses aren't coming out properly. I am watching in shock and awe as "129.173.118.0" and "129.173.31.187" both return 2164260864

Can someone explain what is going on?

Perhaps I am using the parser incorrectly, I am unsure exactly how it works, namely the "%uhh." are new to me and I have no idea as to whatever is going on in that return statement.

This is why you check the return value of `scanf` and friends — M.M, Dec 03 '15 at 22:39
`"%hhu"` the modifier should come first And it should be `unsigned char ipbytes[4];` — wildplasser, Dec 03 '15 at 22:40
Note that these are signed chars, and they get sign-extended to ints. — user253751, Dec 03 '15 at 22:40

M.M · Accepted Answer · 2015-12-03T22:52:54.350

Your sscanf is failing. When this happens, it leaves the arguments in an indeterminate state. So your return statement returns garbage.

%uhh means to read into an unsigned int , and then match the letter 'h' twice. If your input string did not actually contain h after the first number, then matching fails and sscanf will return 1 (or 0 if there wasn't even a first number).

You probably meant %hhu, to read an integer and store in an unsigned char, however you also need to make ipbytes be unsigned char to match the format specifier. The specifier for plain or signed char is %hhd.

This array should be unsigned anyway, otherwise your | later on is going to mess up (remember that negative numbers have a lot of 1 bits set, in 2's complement).

A further problem is that if your system has 32-bit int, then even when using unsigned char, the expression ipbytes[3] << 24 causes undefined behaviour if the high bit of ipbytes[3] is set, due to signed integer overflow: the integer promotions unfortunately promote unsigned char to a signed int.

Finally, if the system has 16-bit int then the << 16 and << 24 fail entirely.

A robust way to write the function would be to avoid any undesirable promotions:

uint32_t parseIPV4string(char const * ipAddress)
{
    unsigned int ip[4];

    if ( 4 != sscanf(ipAddress, "%u.%u.%u.%u", &ip[0], &ip[1], &ip[2], &ip[3]) )
         return 0;   // or some other indicator or error

    return ipbytes[3] + ipbytes[2] * 0x100 + ipbytes[1] * 0x10000ul + ipbytes[0] * 0x1000000ul;
}

If you really want to use | and << then you either have to use some ugly casts; or change ip to have the type uint32_t in which case "%u" must be replaced by "%" SCNu32.

My solution does not detect the case where the input has numbers larger than `255` (it will return junk in that situation) - although your original code didn't handle that well either. To detect this case you could add an extra line , e.g. `if ( (ip[0] | ip[1] | ip[2] | ip[3]) >= 0x100 ) return 0;` — M.M, Dec 03 '15 at 22:57

Converting IP addresses from char * to uin32_t

1 Answers1