[m-rev.] for review: fix float_to_string so that roundtripping works

[Fergus Henderson]

On 21-Nov-2002, Ralph Becket <rafe at cs.mu.OZ.AU> wrote:
> > On 20-Nov-2002, Peter Ross <pro at missioncriticalit.com> wrote:
> > > +:- pragma foreign_code(c, "
> > > +void
> > > +ML_sprintf_float(char *buf, MR_Float f)
> > > +{
> > > +	MR_Float round;
> > > +	int 	 i = ML_MIN_PRECISION;
> > > +
> > > +	do {
> > > +		sprintf(buf, ""%#.*g"", i, f);
> > > +		if (i >= ML_MAX_PRECISION) {
> > > +			/*
> > > +			** This should be sufficient precision to
> > > +			** round-trip any value.  Don't bother checking
> > > +			** whether it can actually be round-tripped,
> > > +			** since if it can't, this is a bug in the C
> > > +			** implementation.
> > > +			*/
> > > +			break;
> > > +		}
> > > +		sscanf(buf, ML_FMT, &round);
> > > +		i++;
> > > +	} while (round != f);
> > > +}
> > > +").
> 
> Small point, but isn't a for loop more appropriate here?
> 
> 	for(i = ML_MIN_PRECISION; i <= ML_MAX_PRECISION; i++) {
> 		sprintf(buf, "%#.*g", i, f);
> 		sscanf(buf, ML_FMT, &round);
> 		if(round == f) {
> 			break;
> 		}
> 	}

That would be OK, but it's probably a little less efficient than the do-while
loop above.  In the case where the number requires the maximum precision,
your loop calls sscanf() and the float equality comparison three times,
whereas the do-while loop executes these only two times.

BTW, it just occurred to me that there is a bug here with regard to the
handling of `inf' and `nan'.  For those, the sscanf() call may fail,
leaving `round' uninitialized, but we go on and access `round',
which results in undefined behaviour.  The uninitialized value may
happen to be a signalling NaN, in which case it would crash.

The simplest solution is to just initialize `round' (e.g. to zero).

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------