Comment On Net Shift

As far as I'm concerned, both Al Gore and Vint Cerf can be fathers of the Internet; it's just that they can't get married in most states. This crazy bastard child we all know and love really did cause a paradigm shift by shrinking the world. When it comes down to it, the Internet is a pile of money and equipment combined with a bunch of standards and peering agreements; if that's not love, I don't know what is. [expand full text]
« PrevPage 1 | Page 2Next »

Re: Net Shift

2007-03-19 09:07 • by M. (unregistered)
BOOL???

Re: Net Shift

2007-03-19 09:08 • by The Frinton Mafia (unregistered)
Eh, he was just fooling around. That's not a REAL WTF.

Re: Net Shift

2007-03-19 09:12 • by Stupidumb
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...

Re: Net Shift

2007-03-19 09:17 • by pinkduck
IsIpAddressZero(0) == false :)

Re: Net Shift

2007-03-19 09:25 • by This is nothing... really... (unregistered)
*ROTFL*

Re: Net Shift

2007-03-19 09:27 • by vt_mruhlin
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!

Re: Net Shift

2007-03-19 09:30 • by RON (unregistered)
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?

Re: Net Shift

2007-03-19 09:37 • by Chris (unregistered)
127315 in reply to 127313
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.

Re: Net Shift

2007-03-19 09:38 • by rbowes
127316 in reply to 127309
Stupidumb:
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...


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!

Re: Net Shift

2007-03-19 09:39 • by s (unregistered)
127317 in reply to 127309
Stupidumb:
Can someone explain why the bit shifts happen?


Shift happens. Do not ask, why.

Re: Net Shift

2007-03-19 09:39 • by Frzr (unregistered)
ROFL... Definitely, one of the best WTFs.

Re: Net Shift

2007-03-19 09:41 • by Spoe (unregistered)
127319 in reply to 127313
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...

Re: Net Shift

2007-03-19 09:46 • by My Name (unregistered)
127320 in reply to 127314
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?

Re: Net Shift

2007-03-19 09:47 • by savar
127321 in reply to 127319
Spoe:
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...


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

Re: Net Shift

2007-03-19 09:48 • by fennec
127322 in reply to 127319
Spoe:
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...

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.

Re: Net Shift

2007-03-19 09:50 • by s (unregistered)
127323 in reply to 127313
vt_mruhlin:
OK, I can look at the bit shifting as a clever idea, but B & 0xFF == B. WTF are those ands for?


0xDEADBEEF && 0xFF = 0x000000EF.
0xDEADBEEF >>8 && 0xFF = 0x000000BE

BOOL IsIpAddressZero(long IP){
return IP == 0;
}
vt_mruhlin:

/Or even better, just check for IP==0 in the place where you'd normally call the function!


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...)

Re: Net Shift

2007-03-19 09:53 • by Richard Smith
127325 in reply to 127320
My Name:
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


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...

My Name:
2) It ANDS together each tier of the address. It then returns this. This has two sub-errors


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.

Re: Net Shift

2007-03-19 09:53 • by fennec
127326 in reply to 127320
My Name:
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
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.
2) It ANDS together each tier of the address.

Wrong. It ORs together each tier. Bitwise.
-It returns 0 if the address is 0
Correct....
or a number !=0
Not correct. All the quads get ORred together. If any are nonzero it will return a nonzero value.
This is backwards from what one would expect a function named IsIPAddressZero() to return.

Very much so.
return IP==0?0:1; is much faster.
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...
You don't even need to convert it from network byte order... ntohl() is called 4 times.
True.

Re: Net Shift

2007-03-19 09:55 • by Tim Smith (unregistered)
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

Re: Net Shift

2007-03-19 09:58 • by vt_mruhlin
127329 in reply to 127325
Richard Smith:

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.


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

Re: Net Shift

2007-03-19 10:01 • by Richard Smith
127330 in reply to 127326
fennec:
My Name:
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
In standard C, an integer can be either 16 bits or 32 bits, depending on the host architecture. A 'long' is always 32 bits


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.

Re: Net Shift

2007-03-19 10:03 • by Richard Smith
127331 in reply to 127329
vt_mruhlin:
Richard Smith:

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.


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


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)
127332 in reply to 127325
You want to evaluate as Boolean the comparison between a Boolean ("IsIpAddressZero(ip)") and an integer literal?

WTF?

Re: Net Shift

2007-03-19 10:07 • by little slash (unregistered)
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!

Re: Net Shift

2007-03-19 10:07 • by Jon (unregistered)
Maybe the four calls to ntohl are for thread-safety..?

Re: Net Shift

2007-03-19 10:09 • by fennec
127335 in reply to 127330
Richard Smith:

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.

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. =)

Re: Net Shift

2007-03-19 10:13 • by s (unregistered)
127336 in reply to 127327
#define TRUE 0

Re: Net Shift

2007-03-19 10:20 • by Greg Lee (unregistered)
127337 in reply to 127326
fennec:
My Name:
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
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.


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.

Re: Net Shift

2007-03-19 10:21 • by Nelle (unregistered)
127338 in reply to 127311
IsIpAddressZero(0) == false :)


#define NULL 0

Re: Net Shift

2007-03-19 10:21 • by mangobrain (unregistered)
127339 in reply to 127334
Jon:
Maybe the four calls to ntohl are for thread-safety..?


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.

Re: Net Shift

2007-03-19 10:22 • by fennec
127340 in reply to 127335
fennec:
Richard Smith:

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.

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. =)

As a followup, I'm reading through the standard (well, skimming quite quickly) and have discovered the following about limits.h:

5.2.4.2.1 Size sof integer types <limits.h>

The values given below shall be replaced by constant
expressions... Their implementation-defined values
shall be equal or greater in magnitude (absolute value)
to those shown, with the same sign.

...

SHRT_MIN -32767 // -(2^15-1)
SHRT_MAX +32767 // 2^15-1
USHRT_MAX 65535 // 2^16-1

INT_MIN -32767 // -(2^15-1)
INT_MAX +32767 // 2^15-1
UINT_MAX 65535 // 2^16-1

LONG_MIN -2147483647 // -(2^31-1)
LONG_MAX +2147483647// 2^31-1
ULONG_MAX 4294967295// 2^32-1


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

Re: Net Shift

2007-03-19 10:23 • by jo42
Imagine the code if the parameter passed was an a.b.c.d formatted string...

Re: Net Shift

2007-03-19 10:25 • by Nelle (unregistered)
127342 in reply to 127331
Richard Smith:
vt_mruhlin:
Richard Smith:

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.


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


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.


Besides each decent compiler throws a warning when comapring/assigning bool and BOOL

Re: Net Shift

2007-03-19 10:29 • by Mcoder
127343 in reply to 127320
My Name:

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

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.

Re: Net Shift

2007-03-19 10:31 • by SomeCoder (unregistered)
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!

Re: Net Shift

2007-03-19 10:40 • by Richard Smith
127346 in reply to 127340
fennec:
fennec:
Richard Smith:

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.

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. =)

As a followup, I'm reading through the standard (well, skimming quite quickly) and have discovered the following about limits.h

snip

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).


I stand corrected. My main objection was to the 'int is 16 or 32 bits' part :)

Re: Net Shift

2007-03-19 10:41 • by akatherder
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
);
}

Re: Net Shift

2007-03-19 10:43 • by vt_mruhlin
127348 in reply to 127342
Nelle:

Besides each decent compiler throws a warning when comapring/assigning bool and BOOL


You should see the code I work with. If I stopped for every warning I saw....

Re: Net Shift

2007-03-19 10:52 • by svnlogger (unregistered)
The real WTF is not knowing who wrote the code, meaning they develop without revision control...

Re: Net Shift

2007-03-19 10:53 • by Jon (unregistered)
127350 in reply to 127339
mangobrain:
Jon:
Maybe the four calls to ntohl are for thread-safety..?


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.

Oops... it was a (fairly bad, admittedly) joke! :)

Re: Net Shift

2007-03-19 10:56 • by fennec
127351 in reply to 127347
akatherder:
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
);
}

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:

sufficiently: The sufficiently modifier, available only on magic compilers, figures out what you need. In particular, a 'sufficiently long int' will hold any value you need to represent, and you can allocate as many 'sufficiently short int' objects as you need.

This otherwise quite useful little article also describes 'too long' and 'too short' and such in a cute little sidebar :)

Re: Net Shift

2007-03-19 10:59 • by Stephen (unregistered)
127352 in reply to 127340
fennec:
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).

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:

BOOL IsIpAddressZero( LONG lIpAddress )
{
return !lIpAddress;
}

Re: Net Shift

2007-03-19 11:29 • by J (unregistered)
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

BOOL IsIpAddressZero( LONG lIpAddress )
{
return ntohl(lIpAddress)) & 0xFFFFFFFF;
}



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)
127356 in reply to 127343
Mcoder:

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.

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).

Re: Net Shift

2007-03-19 11:45 • by MaGnA
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.

Re: Net Shift

2007-03-19 11:51 • by herminator
127359 in reply to 127346
Richard Smith:

I stand corrected. My main objection was to the 'int is 16 or 32 bits' part :)


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.

Re: Net Shift

2007-03-19 11:54 • by My Name (unregistered)
127360 in reply to 127326
Oops.. that's what I get for posting on the internet immediately after I wake up D:

Thank you for the corrections

Re: Net Shift

2007-03-19 12:05 • by N0Nam3 (unregistered)

try {
long foo = 1/ip;
} catch (ArithmeticException ouch) {
return true;
}
return false;


He forgot the powerfull enterprisey goto - exception statement.

Re: Net Shift

2007-03-19 12:16 • by Andrew (unregistered)
127362 in reply to 127316
rbowes:
Stupidumb:
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...


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!


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!

Re: Net Shift

2007-03-19 12:26 • by piersy (unregistered)
The real (or long?) WTF is that noone has mentioned filenotfound yet.
« PrevPage 1 | Page 2Next »

Add Comment