| « Prev | Page 1 | Page 2 | Next » |
|
Eh, he was just fooling around. That's not a REAL WTF.
|
|
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... |
|
*ROTFL*
|
|
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! |
|
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? |
|
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. |
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! |
Shift happens. Do not ask, why. |
|
ROFL... Definitely, one of the best WTFs.
|
|
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...
|
|
What he's doing is very straightforward. There's nothing exceptionally obfuscated about it. However, I see the following things wrong with it:
1) 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 2) 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? |
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 |
No, that's a bitwise or, | and not ||. Basically, he is ORing together all four quads of the IP address and returning that. (This is why there's an & 255 - to restrict this to quad-at-a-time ORing - though one might just as well wait and & everything as the very last operation). Why you would want to do this, now, and moreover call it 'IsIpAddressZero', is the question of the real WTF. |
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...) |
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. |
In standard C, an integer can be either 16 bits or 32 bits, depending on the host architecture. A 'long' is always 32 bits; furthermore, this is the size of a register on a 32-bit machine. Not a WTF - at least not a WTF in this code.
Wrong. It ORs together each tier. Bitwise. Correct.... Not correct. All the quads get ORred together. If any are nonzero it will return a nonzero value.
Very much so. In most cases, using if(IP){ ... } will be even faster still , avoid function call overhead, and do the exact same thing. You might have a case for the above if you absolutely need it to be 1 or 0 so it can be stored somewhere or passed as a 1/0 parameter... True. |
|
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 |
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 |
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. |
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. |
Re: Net Shift
2007-03-19 10:05
•
by
Sgt. Preston
(unregistered)
|
|
You want to evaluate as Boolean the comparison between a Boolean ("IsIpAddressZero(ip)") and an integer literal?
WTF? |
|
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! |
|
Maybe the four calls to ntohl are for thread-safety..?
|
I certainly didn't mean anything like that. I don't program for Windows, for one thing! If I'm wrong, I'm just wrong, and need to review the standards. =) |
|
#define TRUE 0
|
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. |
#define NULL 0 |
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. |
As a followup, I'm reading through the standard (well, skimming quite quickly) and have discovered the following about limits.h:
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 |
|
Imagine the code if the parameter passed was an a.b.c.d formatted string...
|
Besides each decent compiler throws a warning when comapring/assigning bool and BOOL |
On C (it is C, right? I mean, a BOOL function returns an integer...) a long (lower case, but microsoft's library defines a upper case LONG that is equals to long. There is no BOOL either just bool, but microsoft defines it too) has 32 bits, and the size of int changes depending on the plattaform. So here he's right. |
|
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! |
I stand corrected. My main objection was to the 'int is 16 or 32 bits' part :) |
|
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 ); } |
You should see the code I work with. If I stopped for every warning I saw.... |
|
The real WTF is not knowing who wrote the code, meaning they develop without revision control...
|
Oops... it was a (fairly bad, admittedly) joke! :) |
I don't think IPv6 fits in a long. It's 128 bits, no? Even a typical 64-bit 'long long' is probably too short. Maybe you can just wait until they ratify the 'sufficiently' modifier, though:
This otherwise quite useful little article also describes 'too long' and 'too short' and such in a cute little sidebar :) |
int must be at least 16 bits and long must be at least 32 bits. There are systems where they're both 32 bits, systems where they're 32 and 64 bits, and systems where there are no types exactly 16 or 32 bits at all. Signed types can also be ones complement, twos complement, or sign-and-magnitude (which means the possibility that INT_MIN may be -INT_MAX or that -0 exists). Such is the fun of trying to write portable code with C. Of course, the idiomatic way to write this function would be:
|
|
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:
// Returns zero if address is zero Captcha: "atari", where I would have no idea of the size of a long or the byte order. |
Re: Net Shift
2007-03-19 11:34
•
by
Anonymous Coward
(unregistered)
|
No he's not. On just about every 64 bit architecture, 'long' is 64 bits. Which is why you should be using int32_t and int64_t defined in stdint.h if you want to control exactly how many bits you have (this assumes your environment has stdint.h, which is a C99 feature if I'm not totally mistaken). |
|
The _real_ way to check if an IP address is zero:
1. Install additional NICs to all available PCI slots. 2. Sign up for a corporate account with your ISP and assign dedicated IPs to all your NICs. 3. Create a socket and bind it for listening on the IP. 4. Wait for your regional script kiddie to perform random port scans/attacks on your IPs. 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. |
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. |
|
Oops.. that's what I get for posting on the internet immediately after I wake up D:
Thank you for the corrections |
He forgot the powerfull enterprisey goto - exception statement. |
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! |
|
The real (or long?) WTF is that noone has mentioned filenotfound yet.
|
| « Prev | Page 1 | Page 2 | Next » |