Hacker News new | ask | show | jobs
by fshaun 2790 days ago
Yes, agreed. It is a step backward to manipulate and recombine bytes. "Extracting an integer from a packet, then changing it's internal format" is exactly what is being done, and thinking at that abstraction level is beter than playing with individual bytes. And suppose you hoist those operations into a separate function instead of doing it for every field? Well, congratulations on reimplementing ntohX. If the boundary between internal and external data is not clear, that's a bigger issue that could cause incorrect or unnecessary data transformations, whether we're talking about byte order, string sanitizing, unit conversions, etc.
1 comments

The problem is that the higher-level abstraction is broken. Specifically, the abstraction provided here is "extract an integer of unspecified size and endianness from a byte stream" (note that the size of short isn't guaranteed to be anything specifically, other than 2 <= sizeof(short) <= sizeof(int)). And ntohs is another abstraction which amounts to, "make the endianness correct for the current system, assuming that it was originally big-endian". Not that it doesn't work - it's just not well-designed, and too easy to use incorrectly.

A proper abstraction is the one where you specify both the size and the endianness at the point where you extract the value - i.e. where you just call a function like read_int16_be(byte_stream).

> (note that the size of short isn't guaranteed to be anything specifically, other than 2 <= sizeof(short) <= sizeof(int)).

That isn't guaranteed. sizeof(short) and sizeof(int) are both allowed to be 1 on a system with chars at least 16 or 32 bits wide, respectively.

Yep, you're right.

Which just goes to show that these are not the types you want to be using for any kind of portable binary I/O.