Add support for writing 32-bit floats #232
Conversation
ionc/ion_binary.c
Outdated
| iERR ion_binary_write_float_32_value(ION_STREAM *pstream, float value ) | ||
| { | ||
| iENTER; | ||
| uint32_t intvalue = 0; | ||
| BYTE image[ UINT_32_IMAGE_LENGTH ]; | ||
| BYTE *pb = &image[UINT_32_IMAGE_LENGTH - 1]; | ||
| int len; | ||
|
|
||
| ASSERT( UINT_32_IMAGE_LENGTH == 4 ); // we do depend on this here | ||
| ASSERT( UINT_32_IMAGE_LENGTH == sizeof(value) ); // we also depend on this here | ||
| ASSERT( pstream != NULL ); | ||
|
|
||
| // this copy allows us to make endian issues just int issues | ||
| intvalue = *((uint32_t *)&value); | ||
|
|
||
| // write 8 bits at a time into the temp buffer | ||
| // from least to most significant, backwards | ||
| // that is starting from the back of the temp | ||
| // buffer (image) - and here we always write all 8 bytes | ||
| for (len = 4; len; len--) { | ||
| *pb-- = (BYTE)(intvalue & 0xff); | ||
| intvalue >>= 8; | ||
| } | ||
|
|
||
| // now write the bytes out from most to least significant | ||
| // to the output stream | ||
| ASSERT((pb + 1) == image); | ||
|
|
||
| IONCHECK(ion_binary_write_byte_array(pstream, image, 0, UINT_32_IMAGE_LENGTH)); | ||
|
|
||
| iRETURN; | ||
| } |
There was a problem hiding this comment.
The copy-paste here might be worth the macro...
There was a problem hiding this comment.
I was going to say that I disagreed because of the number of parameters it would have to have:
- type of
intvalue - size of image
- expected size of image
Actually that's not so many, after all. Huh...
| iERR _ion_writer_binary_write_float(ION_WRITER *pwriter, float value); | ||
| iERR _ion_writer_binary_write_double(ION_WRITER *pwriter, double value); |
There was a problem hiding this comment.
There is a weird asymmetry between these APIs and the _float32 and _float64 names. I don't feel super strong about it, but I am curious as to your thoughts about this.
There was a problem hiding this comment.
The public interface corresponds to the C types, but the machinery is written in terms of the type of float that's written to the Ion stream. These methods are between the two.
| // See https://amzn.github.io/ion-docs/docs/binary.html#4-float | ||
| // "If L is 0, then the the value is 0e0 and representation is empty." | ||
| test_ion_binary_reader_supports_32_bit_floats((BYTE *) "\xE0\x01\x00\xEA\x40", 5, 0.); |
There was a problem hiding this comment.
I think the zero case is good, but has nothing to do with 32-bit-ness per se...
There was a problem hiding this comment.
I copied the test values from the reader tests, figured that the same set of values was valuable for both. And a zero test for writing was valuable because I learned that we represent that in 1 byte instead of 5 (total length 5 instead of 9).
Issue #, if available: #139
Description of changes: Adds support for writing 32-bit floats via a new method exposed in
ion_writer.h:Also adds unit tests for the same values used to verify 32-bit float read support, and amends the
assertBytesEqualhelper method to yield more useful feedback during failures.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.