Endian Library Do List

Format Review Comments

Votes

John Filo - "Absolutely. I'd like to see support for float and double, but
even without those additions, I still vote yes." "For those who deal with non-native endian data, this library is
extremely useful. It automatically eliminates a whole class of common
programming errors when dealing with such data."

Hartmut Kaiser - "Even if this is not a full review, I would like to vote YES to include this
library into Boost.

Boost.Spirit is using (and shipping) with an older version of this library
for several years now and we never had any problems with its usage in
Spirit. It is used as the underlying framework for the binary parsers and
generators and it is functioning as advertised.

As a quick test I replaced the internal (older) version of Boost.Endian in
Spirit with the reviewed version. All of Spirits regression tests still
pass. "

Robert Stewart - "There are issues that keep me from saying yes at this time. There are too many suggested variations and ideas under consideration to accept the library in its present state. However, a mini-review should be sufficient to evaluate the final form, once Beman determines a course of action, and determine whether to accept it or not."

Tim Blechmann - "the library should be accepted, if

(a) the interface of the conversion functions is changed
(b) the performance can be improved
(c) the documentation integrates better with the rest of the boost
documentation."

Vicente J. Botet Escriba - "No in its current state.
Once the library takes in account the requested modification (that Beman has already accepted) a mini-review will be necessary to improve the library before release."

Executive summary

Common use case scenarios should be developed.

Example programs should be developed for the common use case scenarios.

Documentation should illuminate the differences between endian integer/float type and endian conversion approaches to the common use case scenarios, and provide guidelines for choosing the most appropriate approach for user's applications.

Conversion functions supplying results via return should be provided.

Platform specific performance enhancements such as use of compiler intrinsics or relaxed alignment requirements should be supported.

Endian integer (and floating) types should be implemented via the conversion functions. If that can't be done efficiently, consideration should be given to expanding the conversion function signatures to resolve the inefficiencies.

Benchmarks that measure performance should be provided. It should be possible to compare platform specific performance enhancements against portable base implementations, and to compare endian integer approaches against endian conversion approaches for the common use case scenarios.

Float (32-bits) and double (64-bits) should be supported. IEEE 754 is the primary use case.

Support for user defined types (UDTs) is desirable, and should be supported where there would be no conflict with the other concerns.

There is some concern that endian integer/float arithmetic operations might used inadvertently or inappropriately. The impact of adding an endian_buffer class without arithmetic operations should be investigated.

Stream insertion and extraction of the endian integer/float types should be documented and included in the test coverage.

Binary I/O support that was investigated during development of the Endian library should be put up for min-review for inclusion in the Boost I/O library.

Docs

Document use of endian integers with stream inserters and extractors.

Conversion in note mention similarity to htonl() , etc.

Conversion: add discussion of alignment, packing, etc. Bottom line; use at your own risk. Use Phil's example:
struct S {
uint16_t a;
uint32_t b;
} __attribute__ ((packed));

Requirements for template parameters.

UDTs

Integers
Conversion

Distinguish use cases and recommendations for which approach (integers vs conversion) is appropriate.

http://lists.boost.org/Archives/boost/2011/09/185698.php

> section `experience': this section gives no insights for people who use or
> read the code. it mainly tells: "we are not the first and the domain of the
> library is important.". imo this section can be removed (maybe the part that
> it is not related to any c library can go to the `design considerations'
>
> section "motivating use cases": this is more a marketing blurb/testimonial.
> maybe this could be changed to a section about possible use cases, listing
> `communicating between different devices' and `reading/writing of binary file
> formats'.

one other point ... the help file seems to directly link to the c++ headers.
this should be changed:

* some browsers (at least chromium) will not display the header when clicking
the link, but will save them on disk.

* providing a direct link to the source code from the docs implies that the
user will get some information that are necessary to use the library by
reading the sources. imo, this is not the case for using boost.endian.

* if a user opens integer.hpp, the first 60 lines just contain copyright, some
historical notes, compiler-specific stuff, includes and ifdefs. imo, this is
the implementation part, which should not be exposed to a user.

so i'd suggest to completely remove the links to the c++ headers.

> explaining the other builting types are not considered. Why only big/little
> endianness has been taken in account?

I'll add FAQ and/or add more entries to the final docs.

Only big/little endianness is taken into account because these are the
only endian schemes that have any practical value. All the others are
just historical curiosities.

Code

Also change docs if applicable.

Beman: TODO: Google "unaligned integer", look at various entries. For example, http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.faqs/ka3544.html

Beman: Some platforms (compiler/processor taken together) don't require alignment for the conversion functions if code is inlined, although speed may suffer. (test to verify this assertion). On those platforms, conversion functions (perhaps in-place) can be used to implement unaligned integers.

Microsoft UNALIGNED / __unaligned keywords for pointers. Also Packing Structures.
GCC http://www.mailinglistarchive.com/gcc@gcc.gnu.org/msg21079.html
Intel performs well on simple unaligned test. See ../test/unaligned_test.cpp

Beman: Does the use of the unrolling templates get in the way of processor specific optimization code?

Test use of endian integers with stream inserters and extractors.

Continue work on benchmarks and timings. Consider using use-case example programs as benchmarks.

> The library should provide in addition endian conversion functions that have
> the endiannes as template parameters to make possible generic functions.

Compile time dispatch on an endianness enum was also requested in
another review. That's fine with me, but I haven't had a chance to
figure out the interface details.

Infrastructure

> endian_operations_test.cpp and endian_in_union_test.cpp ... maybe rename from
> _test.cpp to _compile_test.cpp? they don't seem to do any run-time tests. they
> also should not include <cassert> since no assertion statement is needed, this
> might speed up the compilation time of the testsuite by something like 50ms ;)

Will do.

Make the bin() functionality available

* I'm only willing to provide conversion.hpp FP support. Providing
types that mimic FP types is far beyond my knowledge of how to deal
with floating point's notorious arithmetic issues.

Support IEEE754 format (32 bit, 64 bit) only.