On Fri, Jan 23, 2015 at 12:47 PM, Peter Dimov
Joel FALCOU wrote:
Is the library ready to be added to Boost releases?
Let me preface everything else I'll say by that: I think that the answer to the above question is "yes", as every question raised during the review seems to have been addressed.
That said, I want to point out that the library in its current state does not support one approach to dealing with endianness, which I will outline below.
Let's take the following hypothetical file format as an example, loosely lifted from the docs:
[00] code (32 bit big endian) [04] length (32 bit big endian) [08] version (16 bit little endian) [10] shape type (32 bit little endian) [14] ...
All three approaches that the library supports involve declaring a struct with the above members and expecting that this struct can be read/written directly to file, which means that its layout and size must correspond to the above description.
What I tend to do, however, is rather different. I do declare a corresponding struct:
struct header { int code; unsigned length; int version; int shape_type; };
but never read or write it directly, which means that I do not need to make sure that its layout and size are fixed.
Yes, decoupling the external I/O format from the internal format is often good design.
Instead, in the function read(h), I do this (pseudocode):
read( header& h ) { unsigned char data[ 14 ]; fread data from file;
read_32_lsb( h.code, data + 0 ); read_32_lsb( h.length, data + 4 ); read_16_msb( h.version, data + 8 ); read_32_lsb( h.shape_type, data + 10 ); }
Using the endian library to achieve the same effect: struct header // same as above { int code; unsigned length; int version; int shape_type; }; read( header& h ) { struct { big_int32_buf_t code; big_uint32_buf_t length; little_int16_buf_t version; big_int32_buf_ut shape_type; } data; fread data from file; h.code = data.code.value(); h.length = data.length.value(); h.version = data.version.value(); h.shape_type = data.shape_type(); } This seems to me to be better code because the layout of either of the two structs can change without having to change anything else. In your code, if the size of " [00] code (32 bit big endian)" changed to " [00] code (64 bit big endian)", then all four of the read_* function calls would have to be changed. --Beman