Re: [boost] [endian] Some suggestions
1. Bring back float: make a proper reason for abandon You told about NaN: but it shouldn't apply since you store it in char buffer.
The problem isn't when storing into the char buffer, but later on a different machine or compiler when the char buffer has to be converted to a float type.
I know nothing other than float format differences, and my target platforms are using iec559. I think reinterpret_cast should work: char data shouldn't be changed as long as stored as char.
In testing, I even ran into a case were on the same machine and compiler the float data failed to round-trip correctly if the program that created the file was compiled with a different set of optimization options that the program the read the file.
I will be happy if you provide some example.
2. Remove n_bits(endian buffers): completely useless This just look like an alternative to sizeof, and always /8'd when used. What's the meaning of that?
Please give a specific code example from the docs of what you think should be changed. Or submit a pull request.
Since this is breaking API, I'd like you to make the decision to
change it or not.
For detail, this is the endian_buffer definition:
template
3. Minor API suggestions Move operator type to endian_buffer: it's not related to arithmetic but just used because it can act like an integer.
Again, what code is it you think should be changed? Operator value_type()? Please submit a pull request.
Exactly: operator value_type(). This will provide much more convenience. An example: when you specify the size for the array: endian_buffer size; dynarray d(size.value()); vs dynarray d(size);
Non-const data(): I'm tired to do reinterpret_cast. Look at C++11 vector for a reference.
What is the motivation? What are the safety implications? What are the alternatives? The C++11 (and also C++17) addition of non-cost data() members to some standard library containers was supported by motivation and safety analysis.
Seems endian_buffer is targeted for using in structs. Then is there any problem to provide a wrap of reinterpret_cast? At least, it should be safe by design.
Move stream operators to endian_arithmetic: I first thought that was for reading binary. That's basically unused.
I must be missing something. Why wouldn't stream operators apply to endian buffers? A code example might help.
Quite confusing. I first thought that was binary r/w operators that reads/writes the stream and store the value. The problem is that, there's no method provided for reading from binary, so this is also a reason for the previous data() suggestion.
On Mon, Apr 11, 2016 at 8:54 AM, Tatsuyuki Ishi
1. Bring back float: make a proper reason for abandon You told about NaN: but it shouldn't apply since you store it in char buffer.
The problem isn't when storing into the char buffer, but later on a different machine or compiler when the char buffer has to be converted to a float type.
I know nothing other than float format differences, and my target platforms are using iec559. I think reinterpret_cast should work: char data shouldn't be changed as long as stored as char.
Yes. I should have said that the problem isn't when storing into a char buffer, as is done by the endian_buffer and endian_arithmetic classes, but in the conversion functions where possibly byte-swapped values are passed as float or double: double endian_reverse(double); That's the interface I'm concerned about because of the return of a double that in fact is just an array of 4 bytes.
In testing, I even ran into a case were on the same machine and compiler the float data failed to round-trip correctly if the program that created the file was compiled with a different set of optimization options that
the
program the read the file.
I will be happy if you provide some example.
See my reply to Peter Dimov.
2. Remove n_bits(endian buffers): completely useless This just look like an alternative to sizeof, and always /8'd when
used.
What's the meaning of that?
Please give a specific code example from the docs of what you think should be changed. Or submit a pull request.
Since this is breaking API, I'd like you to make the decision to change it or not.
For detail, this is the endian_buffer definition: template
class endian_buffer; n_bits looks quite useless here. It's basically sizeof*8.
The size has to be supplied as either the size in bytes or the size in bits. Remember that these classes support 3, 5, 6, and 7 byte integers as well as the more usual 1, 2, 4, and 8 byte integers.
3. Minor API suggestions Move operator type to endian_buffer: it's not related to arithmetic but just used because it can act like an integer.
Again, what code is it you think should be changed? Operator
value_type()?
Please submit a pull request.
Exactly: operator value_type(). This will provide much more convenience. An example: when you specify the size for the array: endian_buffer size; dynarray d(size.value()); vs dynarray d(size);
The rationale for a separate endian_buffer class was that some formal review comments were very concerned about the expense of conversions in class endian_arithmetic. They asked for a separate endian_buffer class that did not perform arithmetic and other automatic conversions. To add automatic conversions to endian_buffer would partially negate the whole purpose of a separate endian_buffer class.
Non-const data(): I'm tired to do reinterpret_cast. Look at C++11
vector
for a reference.
What is the motivation? What are the safety implications? What are the alternatives? The C++11 (and also C++17) addition of non-cost data() members to some standard library containers was supported by motivation and safety analysis.
Seems endian_buffer is targeted for using in structs. Then is there any problem to provide a wrap of reinterpret_cast? At least, it should be safe by design.
I worry about buffer overflows. (I also worried about that problem for the standard library. Only time will tell if that worry is well founded.)
Move stream operators to endian_arithmetic: I first thought that was
for
reading binary. That's basically unused.
I must be missing something. Why wouldn't stream operators apply to endian buffers? A code example might help.
Quite confusing. I first thought that was binary r/w operators that reads/writes the stream and store the value.
The problem is that, there's no method provided for reading from binary, so this is also a reason for the previous data() suggestion.
I agree there is a need to better handle binary I/O, but it seems to me that is a problem with istream/ostream from the standard library rather than the endian library. Thanks, --Beman
participants (2)
-
Beman Dawes -
Tatsuyuki Ishi