| |||||||||||||
Quotes of the weekQuotes of the weekPosted Apr 5, 2012 20:30 UTC (Thu) by samroberts (subscriber, #46749)In reply to: Quotes of the week by RobSeace Parent article: Quotes of the week
> They are just abstracting away the ugly #ifdef
Again, Rob's point was that writing code conditional on the *host's* byte-order is unnecessary for dealing with external data.
libc doesn't necessarily have an ifdef, ntoh* can be written in portable code with no ifdef. Plan9's libc, according to Rob, had no such ifdef in it.
> I've never seen anyone do anything that nutty...
https://github.com/sam-github/libnet/blob/master/libnet/s...
Now you have. And no, I didn't write that.
(Log in to post comments)
Quotes of the week Posted Apr 5, 2012 21:00 UTC (Thu) by RobSeace (subscriber, #4435) [Link]
> libc doesn't necessarily have an ifdef, ntoh* can be written in portable
> code with no ifdef. Plan9's libc, according to Rob, had no such ifdef in it.
I suppose so... But, glibc at least does this:
uint16_t
And, then defines ntohs() as the same thing... And, htonl()/ntohl() is the same but with __bswap_32() of course...
Which I believe is exactly what he was talking about... So, like I said, you're merely abstracting away the ugliness... (Which isn't a bad thing in any way! If you need to have ugliness, better to hide it away...)
> Now you have.
Ok, that's just stupid... And, unless I'm missing something, it does absolutely nothing... Because on big-endian, htons() is effectively a no-op function... So, it seems to me it's doing the same direct assignment on both big and little-endian systems... Perhaps they wanted the reverse test? Which would then be merely redundant (since htons() effectively does it for you already) rather than flat out insane...
Anyway, that seems less like someone doing byte-swapping when not needed, but more just someone fundamentally misunderstanding how htons() works... Maybe they thought it was always a byte-swap routine, and didn't realize that on big-endian systems it's going to be a no-op? That would make their code make some sense (they're trying to force it to little-endian always)...
Quotes of the week Posted Apr 5, 2012 23:30 UTC (Thu) by viro (subscriber, #7872) [Link]
Egads... OK, here's the deal: anybody thinking in terms of "swapped" and "native" is bloody wrong. Doing it that way is *the* recipe for hard to find bugs. There are 3 things: native, little-endian and big-endian. That's what one ought to keep track of, not "have I done byteswap odd or even number of times". Forget about the fact that conversions are involutions. Sure, on any realistic platform it will be true that htonl(x) is always equal to ntohl(x). Which is not interesting for anything other than low-level implementation of those primitives. bswap32 is the wrong thing to do outside of the internals of their implementation.
BTW, code doing n = ntohl(n) is asking for trouble, period. Don't do that - if I ever see that kind of crap in patches touching fs/*, you can be sure that you'll get it back with unkind comments. Don't reuse variables between e.g. native and big-endian; sooner or later you'll get confused and do something really dumb like mismatched number of flips on different paths. And that's not just a theory - I've seen quite a few bugs of that kind.
uint32_t is _not_ "one of {__le32,__be32}, depending on host endianness". It's a separate type and should be treated as such. If the variable can be reused, leave doing that to compiler.
Having the same struct used both with host- and fixed-endian contents is a really bad practice; better split it in two types and take care about which one is getting used in any particular place. And no, "I'll just convert these fields in place here" is *not* a good idea. Been there, fixed quite a few of resulting bugs. Hadn't been fun at all...
Quotes of the week Posted Apr 6, 2012 0:51 UTC (Fri) by RobSeace (subscriber, #4435) [Link]
Did you mean to reply to my comment? Because I agree with everything you said... If I gave any impression otherwise, I apologize... But, I'm confused as to where I gave that impression...
|
|||||||||||||