- 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
As approaches go, it's not so bad. Convert the ends of the range from string-form to what is effectively
uint32_t
(specificallyin_addr
) in host order, compare the twoin_addr
s to make sure that the range is the right way round, then enumerate the range to produce individual IPs.The loop-end condition makes my teeth itch a bit, but sure.
It just makes me wonder what the original code looked like.
Admin
I feel like the second sentence should have either an "only" or a "not".
Admin
I'd have thought the obvious thing to do would be to check for utilities on CPAN. And as far as I can remember, Net::IP is the bees' knees for a requirement like this.
Isn't that the first thing that lazy (in a good way) Perl programmers do?
Admin
Like "Sole Purpose Of Visit" said the obvious thing to do is use a library. Any of the HLL scripting languages are going to have a few good choices of libs for doing basic ipv4 handling by now.
What I find most amazing is and often a huge source of WTFrey is people that insist on playing with IPs as a strings. In my view the first thing you should do with IP addresses and masks is convert them to their integer (ok maybe struct in_addr ) representations. That way any manipulation, range/identity checks, iterating you need to do become trivial bitwise operations. Which is of course how the entire addressing scheme was designed to work. Convert them back to doted decimal at display/logging time if there is a display/logging time.
Admin
As far as IP manipulation goes, this is a pearl.
Admin
The biggest problem I think is that not every
uint32_t
is a valid IP address. In particular, if you take the subnet mask, invert all the bits and then or it with the subnet, you get the broadcast address which is the address used to send a message to every IP address in the subnet. So, for example, with a /24 subnet, setting the last byte to 256 will send the packet to every host in the subnet.Admin
Are you suggesting that it would be better as a rube-y?
Admin
Not every 32 bit number, signed or not, is a valid IP address. But the broadcast address is a valid one, in fact, as you pointed out, is the one you use to send to all the hosts in the subnet. It will NOT send to every IP address, but it will be received by every host, like if every one of them was configured with it ( and they will know it was sent to the broadcast address, and receive only one copy per interface, no matter how many address they have configured in it ). Also, you cannot set an ( 8 bit ) byte to 256, 255 is the one for broadcast, 123.456.789.255 will be a television series broadcast address ( they use invalid IPs like they use the 555 phone numbers ).
Admin
Admin
Are you suggesting that broadcast addresses are not valid IPs?
Admin
You get one of the broadcast addresses. There are three easy ones:
Nobody much uses the second one any more, but it is why you shouldn't use netpart.allzeroes as a local (unicast) IP address.
And they are valid IP addresses. 255.0.0.0 to 255.255.255.254 probably aren't valid, though. Or at least they aren't usable for anything.
Not really. 255, maybe, but not 256.
Admin
Pardon the humble question, but ... where's the WTF?
As Hal says, the first thing you should do is convert to integer. That's what the code does. The only string processing is what's necessary for integer conversion, i.e. splitting on dots.
Yes, it uses a regex, that's because Perl's split() requires one. (I'm not even a Perl programmer, but that's easy to google.) It's a single escaped dot, exactly what you need here, and trivial to read (which means something for a regex ;).
The integer conversion might be a little more readable with shifts rather than multiplication, but that's hardly a WTF.
The end condition is written a bit clumsy, as Steve_The_Cynic says, but still correct.
Other comments discuss broadcast addresses, which is all nice and well, but without knowing what the code does with the addresses, iterating over a range that includes broadcast addresses may be just fine.
In particular, there seems nothing specific to /24 networks, or anything about netmasks at all. In case you think, treating IPv4 addresses as 4 bytes is somehow related to classful networks, err, nope!
OK, a local variable is called "@Class". Is that what makes you think it's about classful networks, so much as to inspire the article title? (And FWIW, since the variable apparently just contains the 4 bytes again, calling it "class" would be wrong even back then.)
Oh, I get it. It uses the readable "if ... die" form instead of the idiomatic Perl "die ... unless not". Truly a WTF!
Admin
I agree, where's the WTF?
For me, the WTF is using the string comparison operator
ne
instead of the appropriate operator for numbers:!=
.In fact, I'd remove the
+1
and just compare using<=
. (I know it can break if the integer overflows, or if the variable turns into a float.) Or replace thewhile
with afor
loop.