[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