[illumos-Developer] Review for 166 CR6901979 error in xdr_float.c not fixed

[Jason King]

I have tested with i386 and amd64 (using the test program from the
original email).  However, I don't have any access to sparc equipment
to validate.  If someone can assist with that (would need to run
nightly + test program), please let me know.

On Sat, Jan 22, 2011 at 4:09 PM, Garrett D'Amore <garrett at damore.org> wrote:
> 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
>
>
>