- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
BOOL???
Admin
Eh, he was just fooling around. That's not a REAL WTF.
Admin
Can someone explain why the bit shifts happen? I assume the WTF is that he could have just checked if it equaled 0, rather than doing a bitwise AND. If that doesn't make sense, look at my name...
Admin
IsIpAddressZero(0) == false :)
Admin
ROTFL
Admin
OK, I can look at the bit shifting as a clever idea, but B & 0xFF == B. WTF are those ands for? Not to mention that it returns false if IpAddessIsZero. Not the best name.
BOOL IsIpAddressZero(long IP){ return IP == 0; }
/Or even better, just check for IP==0 in the place where you'd normally call the function!
Admin
It looks like the original coder was using the "Job Protector" design pattern.
You know, the one where the code he writes is specially obfuscated so that no one can maintain it but him?
Admin
But as he's working with longs, for values of 'B' greater than 255: B & 0xFF != B So those ands are for carefully (and pointlessly) testing each byte for zeroness.
Admin
The reason for the bit-shifts is likely due to somebody memorizing the difference between little- and big-endian, who wanted to make sure that the bytes were checked in the correct order. That also explains the ntohl() calls -- brute force ignorance!
Admin
Shift happens. Do not ask, why.
Admin
ROFL... Definitely, one of the best WTFs.
Admin
Of course, that doesn't replicate the function. It seems to be checking if any one of the four quads in the the dotted quad notation of the address are zero, not if the whole address is zero. Why he'd've wanted to do that, OTOH...
Admin
What he's doing is very straightforward. There's nothing exceptionally obfuscated about it. However, I see the following things wrong with it:
Parameter type is a LONG. IP addresses fit into an integer. I don't know if this is some language-specific thing, but it's missing being stored in a register
It ANDS together each tier of the address. It then returns this. This has two sub-errors
-It returns 0 if the address is 0, which is false, or a number !=0, which is true. This is backwards from what one would expect a function named IsIPAddressZero() to return.
-Simply doing:
return IP==0?0:1;
is much faster. You don't even need to convert it from network byte order.
3)ntohl() is called 4 times. The value should have been calculated once and stored in a variable (were the stubborn programmer stick with this method)
Anyone else see anything?
Admin
I just got to work, and I haven't even finished my first cup of joe, but it seems to me that this function returns true if any byte in the IP address is non-zero -- which of course has nothing to do with the name of the function:
IP = 0.0.0.0 -> FALSE IP = 255.255.255.255 -> TRUE IP = 10.0.0.0 -> TRUE
Admin
Why you would want to do this, now, and moreover call it 'IsIpAddressZero', is the question of the real WTF.
Admin
0xDEADBEEF && 0xFF = 0x000000EF. 0xDEADBEEF >>8 && 0xFF = 0x000000BE
BOOL IsIpAddressZero(long IP){ return IP == 0; }
Oh, sure, but that's a non-pretty, hackish exploitation of a completely accidential and unrelated coincidence that bitwise low-endian 0 == high-endian 0. In case the specific IP representation of the value of "0.0.0.0" gets represented by a pattern of bits different than '00000000000000000000000000000000' your comparison would fail. Using "ntohl()" assures this wouldn't happen in his case!
(I bet this guy read somewhere that NULL is not always =0, and it was knocking around his head...)
Admin
Um, LONG is a type of integer. But perhaps maybe you meant the type int? Clearly this code is written to be portable (it calls ntohl), and there's no guarantee that int is at least 32 bits...
No it doesn't. It ORs together each quad. This means you can do this:
bool mightBeLocalhost = (IsIpAddressZero(ip) == 127);
and other such monstrosities, should you be barking mad.
Admin
Admin
Aside from the reversed logic (0 = FALSE instead of TRUE), this routine will kill anybody silly enough to write the following code:
IsIPAddressZero (address) == TRUE
Admin
IsIpAddress returns a BOOL. If we're to assume BOOL == bool (which might not be an assumption we can make at this point....)... true != 127 && false != 127
Admin
Wrong! All that's guaranteed is that 'long' is at least as large as 'int'. Perhaps you meant that a LONG is defined by standard Windows headers to always be 32 bits. I don't know whether that's true or not.
Admin
BOOL is almost never used as a typedef or #define for bool. It's usually used as a typedef for char or int. See, for instance, the standard Windows headers.
Admin
You want to evaluate as Boolean the comparison between a Boolean ("IsIpAddressZero(ip)") and an integer literal?
WTF?
Admin
BOOL IsIpAddressZero( LONG lIpAddress ) { return ( (ntohl(lIpAddress)>>31) & 1 | (ntohl(lIpAddress)>>30) & 1 | (ntohl(lIpAddress)>>29) & 1 | (ntohl(lIpAddress)>>28) & 1 | (ntohl(lIpAddress)>>27) & 1 | (ntohl(lIpAddress)>>26) & 1 | (ntohl(lIpAddress)>>25) & 1 | (ntohl(lIpAddress)>>24) & 1 | (ntohl(lIpAddress)>>23) & 1 | (ntohl(lIpAddress)>>22) & 1 | (ntohl(lIpAddress)>>21) & 1 | (ntohl(lIpAddress)>>20) & 1 | (ntohl(lIpAddress)>>19) & 1 | (ntohl(lIpAddress)>>18) & 1 | (ntohl(lIpAddress)>>17) & 1 | (ntohl(lIpAddress)>>16) & 1 | (ntohl(lIpAddress)>>15) & 1 | (ntohl(lIpAddress)>>14) & 1 | (ntohl(lIpAddress)>>13) & 1 | (ntohl(lIpAddress)>>12) & 1 | (ntohl(lIpAddress)>>11) & 1 | (ntohl(lIpAddress)>>10) & 1 | (ntohl(lIpAddress)>>9) & 1 | (ntohl(lIpAddress)>>8) & 1 | (ntohl(lIpAddress)>>7) & 1 | (ntohl(lIpAddress)>>6) & 1 | (ntohl(lIpAddress)>>5) & 1 | (ntohl(lIpAddress)>>4) & 1 | (ntohl(lIpAddress)>>3) & 1 | (ntohl(lIpAddress)>>2) & 1 | (ntohl(lIpAddress)>>1) & 1 | ntohl(lIpAddress) & 1 ); }
catcha: fixed!
Admin
Maybe the four calls to ntohl are for thread-safety..?
Admin
Admin
#define TRUE 0
Admin
Not quite right there. An "int" in C is defined to be a register sized value which can vary depending on the architecture.
A "long" is defined to be a data type able to store integer values in at least in the range [-2147483648, 2147483647] or 32-bit signed numbers. However, the type can be larger. The TI compiler for the DSPs they produce has long as a 40-bit type because the processor has support for 40-bit arithmetic.
Admin
#define NULL 0
Admin
They had better not be, since first of all it still wouldn't give you any (the function isn't returning a pointer to local storage, so any hypothetical thread-unsafety must reside inside ntohl() itself), and second, you can still be thread-safe whilst only calling the function once. Bottom line: ntohl() isn't going to return different results when called four times with the same input parameter.
No offence, but you'd only think of doing this in the interests of thread safety if you didn't understand the concept.
Admin
This appears to be from C99, Technical Corrigenda 2. It seems to support the 16/32-bittiness assumption I was making earlier, anyway, at least as minimum values (it appears any of these data types can be longer than this, though).
edit removed some redundant labeling for brevity
Admin
Imagine the code if the parameter passed was an a.b.c.d formatted string...
Admin
Besides each decent compiler throws a warning when comapring/assigning bool and BOOL
Admin
Admin
I could be wrong, but this code looks like it was designed to run on Windows what with the BOOL and all.
Therefore, I'd guess portability isn't a primary concern.
I could be completely off the wall though. Even if I am....
My eyes! The goggles do nothing!
Admin
I stand corrected. My main objection was to the 'int is 16 or 32 bits' part :)
Admin
IPv6 compliant version
BOOL IPv6_IsIpAddressZero( LONG lIpAddress ) { return ( (ntohl(lIpAddress)>>40) & 0xFF | (ntohl(lIpAddress)>>32) & 0xFF | (ntohl(lIpAddress)>>24) & 0xFF | (ntohl(lIpAddress)>>16) & 0xFF | (ntohl(lIpAddress)>> 8) & 0xFF | (ntohl(lIpAddress)>> 0) & 0xFF ); }
Admin
You should see the code I work with. If I stopped for every warning I saw....
Admin
The real WTF is not knowing who wrote the code, meaning they develop without revision control...
Admin
Admin
Admin
Of course, the idiomatic way to write this function would be:
Admin
Suppose that you know the data comes from a network stream, but for reasons beyond your control it's blindly loaded into a long integer, you might want to defend against the unknown byte order and the unknown size of a long. Returning zero for success isn't so unusual, but could do with a comment for the casual reader. Still the code is a bit cack, and I'd suggest:
Captcha: "atari", where I would have no idea of the size of a long or the byte order.
Admin
Admin
The real way to check if an IP address is zero:
Verification: If the IP was really zero the listening socket should be listening on all NICs and eventually all the NICs should receive connection attempts of some sorts.
Admin
Keep standing, because your main objection is also wrong. ;-)
In C, the int type is generally equal to a machine word, so it can be 16 bit as well as 32 bits. The reason for this is to prevent the performance hit you get for using two machine words. Using int guarantees that you use one machine word and therefor never incurs this performance hit.
Admin
Oops.. that's what I get for posting on the internet immediately after I wake up D:
Thank you for the corrections
Admin
He forgot the powerfull enterprisey goto - exception statement.
Admin
The ntohl() calls are even more senseless here. The coder is masking all but a byte of the long, and bytes don't change "endianess".
A greater WTF is the widespread use of Ix86 computers today, after building the internet on big-endian Unix computers. We have all these Ix86 based Linux servers, & every "long int" has to swapped, sent over the internet, and swapped back to little-endian!
Admin
The real (or long?) WTF is that noone has mentioned filenotfound yet.