[illumos-Developer] Review for 166 CR6901979 error in xdr_float.c not fixed
Garrett D'Amore
garrett at damore.org
Sat Jan 22 14:09:53 PST 2011
Generally I like the cleanup. I'd like confirmation of correct
execution on all the platforms that are relevant to us:
sparcv8
sparcv9
i386
amd64
Once we are certain that the code works as it should across all of these
platforms, I think you should move ahead with an RTI -- assuming nobody
spots anything that I've missed.
- Garrett
On Sat, 2011-01-22 at 12:59 -0600, Jason King wrote:
> Webrev is at http://cr.illumos.org/view/ltia0hr8/
>
> This does a couple of things:
> 1. Removes the code for a VAX as it seems incredibly unlikely
> Illumos will be ported to such an old CPU. I also suspect there was
> probably some bugs in the code anyway.
> 2. Changes the selection of the XDR float conversion routines based
> on feature (i.e. IEEE 794) instead of platform. Currently there is an
> IEEE 794 implementation and a (slower) platform independent one. When
> the platform independent code is chosen, a compiler warning is
> generated to assist porting.
> 3. Separates the conversion routines into separate functions by
> implementation. The old code had lots of #ifdefs scattered throughout
> the code, making it harder to read. Instead a few #ifdefs at the
> very top of the file pick the implementation to use.
>
> Having said all that it appears the actual conversion bug was somehow
> fixed accidentally in the past. I tried writing a straight float
> value:
>
> FILE *f = fopen("test", "w+);
> float a = 0.0F;
> fwrite(&a, sizeof (a), 1, f);
>
> Then reading it back via xdr_float, and it came back with '0.0' (both
> when compiled as 32 & 64 bit). I've attached the test case (it's very
> small) if anyone can spot any issues with the test. This is largely
> based on the test case documented in the original Sun/Oracle bug.
>
> Regardless, I think there is value in the elimination of dead code and
> improved clarity. The proposed change produces identical results and
> nightly compiles clean.
> _______________________________________________
> Developer mailing list
> Developer at lists.illumos.org
> http://lists.illumos.org/m/listinfo/developer
More information about the Developer
mailing list