5
\$\begingroup\$

Faced with converting floating-point values from a sensor obtained to string on an embedded system to transmit over UART, I came up with the following dbl2str() to handle either float or double input. The accuracy of the last digit in the fractional part wasn't important as the floating point-values were from a temperature sensor on an MSP432. The intent was to avoid loading stdio.h and math.h.

The double value, an adequately sized buffer and then precision for the fractional-part are parameters to the function:

/**
 *  convert double d to string with fractional part
 *  limited to prec digits. s must have adequate
 *  storage to hold the converted value.
 */
char *dbl2str (double d, char *s,  int prec);

The approach is:

  • Handle 0.0 case where integer-part is '0' and pad fractional part to prec '0's, return at that point.
  • Save sign flag (1-negative, 0-posititve), set padding variable zeros equal to prec, change sign of floating-point value to positive if negative.
  • Nul-terminate temp string and fill from end with fractional-part conversion, subtracting 1 from zeros on each iteration, and after leaving conversion loop, pad to remaining zeros.
  • Add separator '.' and continue to fill temp string with integer-part conversion.
  • if sign add '-' to front of temp string.
  • copy temp string to buffer and return pointer to buffer.

(note: the range of floating-point values is from roughly -50.0 to 200.00 so INF was not protected against, nor was exhausting of the 32-byte buffer a consideration)

The code with test case is:

#include <stdio.h>
#include <stdint.h>

#define FPMAXC 32

/**
 *  convert double d to string with fractional part
 *  limited to prec digits. s must have adequate
 *  storage to hold the converted value.
 */
char *dbl2str (double d, char *s,  int prec)
{
    if (d == 0) {                                   /* handle zero case */
        int i = 0;
        *s = '0';                                   /* single '0' for int part */
        s[1] = '.';                                 /* separator */
        for (i = 2; i < 2 + prec; i++)              /* pad fp to prec with '0' */
            s[i] = '0';
        s[i] = 0;                                   /* nul-terminate */
            
        return s;
    }

    char tmp[FPMAXC], *p = tmp + FPMAXC - 1;        /* tmp buf, ptr to end */
    int sign = d < 0 ? 1 : 0,                       /* set sign if negative */
        mult = 1;                                   /* multiplier for precision */
    unsigned zeros = prec;                          /* padding zeros for fp */
    uint64_t ip, fp;                                /* integer & fractional parts */

    if (sign)                                       /* work with positive value */
        d = -d;

    for (int i = 0; i < prec; i++)                  /* compute multiplier */
        mult *= 10;

    ip = (uint64_t)d;                               /* set integer part */
    fp = (uint64_t)((d - ip) * mult);               /* fractional part to prec */

    *p = 0;                                         /* nul-terminate tmp */

    while (fp) {                                    /* convert fractional part */
        *--p = fp % 10 + '0';
        fp /= 10;
        if (zeros)                                  /* decrement zero pad */
            zeros--;
    }
    while (zeros--)                                 /* pad reaming zeros */
        *--p = '0';
    *--p = '.';

    if (!ip)                                        /* no integer part */
        *--p = '0';
    else
        while (ip) {                                /* convert integer part */
            *--p = ip % 10 + '0';
            ip /= 10;
        }

    if (sign)                                       /* if sign, add '-' */
        *--p = '-';

    for (int i = 0;; i++, p++) {                    /* copy to s with \0 */
        s[i] = *p;
        if (!*p)
            break;
    }

    return s;
}

int main (void) {
    
    char buf[FPMAXC];
    double d = 123.45678;
    
    printf ("% 8.3lf  =>  %8s\n", d, dbl2str (d, buf, 3));
    
    d = -d;
    printf ("% 8.3lf  =>  %8s\n", d, dbl2str (d, buf, 3));
    
    d = 0.;
    printf ("% 8.3lf  =>  %8s\n", d, dbl2str (d, buf, 3));
    
    d = 0.12345;
    printf ("% 8.3lf  =>  %8s\n", d, dbl2str (d, buf, 3));
    
    d = -d;
    printf ("% 8.3lf  =>  %8s\n", d, dbl2str (d, buf, 3));
    
    d = 123.0;
    printf ("% 8.3lf  =>  %8s\n", d, dbl2str (d, buf, 3));
    
    d = -d;
    printf ("% 8.3lf  =>  %8s\n", d, dbl2str (d, buf, 3));
}

The function does what I intended, but would like to know if there are any obvious improvements that can be made with a slight-eye on optimization.

Program Output

./bin/dbl2str
 123.457  =>   123.456
-123.457  =>  -123.456
   0.000  =>     0.000
   0.123  =>     0.123
  -0.123  =>    -0.123
 123.000  =>   123.000
-123.000  =>  -123.000
\$\endgroup\$
7
  • 3
    \$\begingroup\$ Am I missing something here? It seems that sprintf() does exactly this job already. Though I'd recommend snprintf() rather than just telling the caller "s must have adequate storage to hold the converted value", since callers can't be trusted. \$\endgroup\$ Jan 30 at 11:43
  • \$\begingroup\$ Are you confident that your few test cases cover all interesting cases? They seem to be too few to me. Have a look at the Go standard library fore some more interesting numbers. \$\endgroup\$ Jan 30 at 13:28
  • \$\begingroup\$ pi*10E19 overflowed, requiring a painful check on p >= 0. Returning the modified passed array is double. The bounds of the passed array cannot be checked. With buffer overflow exploits, please ensure this rests academic code. Use bool for sign. \$\endgroup\$
    – Joop Eggen
    Jan 30 at 14:22
  • 1
    \$\begingroup\$ You've written this because you want to avoid including <stdio.h>. Note that including that file, on its own, should not have any performance or memory impact since it's just function signatures; the impact comes at the link stage when you actually use a function from it. Have you profiled the difference between using your function and using an ftoa (if implemented) or sprintf? \$\endgroup\$
    – Reinderien
    Jan 30 at 14:36
  • 1
    \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. Please post a new question instead. Feel free to add a link back and forth between the questions. \$\endgroup\$
    – Mast
    Jan 31 at 9:07
3
\$\begingroup\$

The intent was to avoid loading stdio.h and math.h.
if there are any obvious improvements that can be made with a slight-eye on optimization.

Consider float rather than double

Avoid splitting string processing

Separate processing for integer part and fraction not needed. A simple alternative is to create a scaled integer and then process that integer "right to left" (least to most).

mult type

A limiting factor is the type of width. Code uses int, which is 16-bit on some embedded machines. To match the rest of codes wide integer type usage, uint64_t mult makes more sense.

Offload padding

dbl2str(double d, char *s, int prec) might as well handle space padding, thus allowing a simple puts() rather than printf ("%8s\n", dbl2str (d, buf, 3));

Such as

dbl2str(double d, char *s, int width, int prec)

Minor: Parameter order

Maybe instead of double d, char *s, int prec, follow the sprintf() order char *s, int prec, double d as a more familiar idiom.

Rounding

The code cost to do basic rounding is not high. I recommend it.

Temperature and -0.000

When reporting temperature, seeing -0.0 can be informative.

Consider using its potential appearance with a signbit(d) test rather than d < 0, or due to rounding.

Size limited string

Early in a project, data is often not what one thinks. A double to string function that uses buffer overflow protection would pay for itself in reduced debugging - better than risk UB.

I did not see a need for special zero handling.

See do loop below.


Some of the above ideas with a modified OP's code

#include <assert.h>
#include <math.h>
#include <stdio.h>
#include <stdint.h>
#include <string.h>
#include <stdbool.h>

#define FPMAXC 32

char* dbl2str2(size_t n, char s[n], int width, int prec, double d) {
  assert(prec >= 0 && prec <= 9); // Or some other upper bound
  assert(width >= 0 && (unsigned ) width < n);
  char tmp[FPMAXC];
  char *p = tmp + FPMAXC - 1;
  *p = '\0';

  // Or use conventional code for signbit, fabs
  bool sign = signbit(d);
  int w = sign + 1; // count characters used: sign, ','
  d = fabs(d) * 2.0; // * 2 for rounding
  for (int p = 0; p < prec; p++) {
    d *= 10.0;
  }

  uint64_t i64 = (uint64_t) d;
  i64 = (i64 + i64 % 2) / 2; // round
  do {
    if (prec-- == 0) {
      *(--p) = '.';
    }
    if ((unsigned) ++w >= n) {
      *s = 0; // Number too big - add error code here as desired.
      return s;
    }
    *(--p) = (char) (i64 % 10 + '0');
    i64 /= 10;
  } while (prec >= 0 || i64);

  if (sign) {
    *(--p) = '-';
    // w++ counted above
  }
  while (w++ < width) {
    *(--p) = ' ';
  }
  return memcpy(s, p, (size_t) (tmp + FPMAXC - p));
}

Output (with "%8s" changed to "%s" and #define dbl2str( d, s, prec) dbl2str2(sizeof(s), (s) , 8, (prec), (d)))

 123.457  =>   123.457
-123.457  =>  -123.457
   0.000  =>     0.000
   0.123  =>     0.123
  -0.123  =>    -0.123
 123.000  =>   123.000
-123.000  =>  -123.000

Code not heavily tested, yet good enough to give some alternative ideas.

\$\endgroup\$
6
  • \$\begingroup\$ Ah hah! Much improved. I had thought about the rounding issue, but didn't have time to devote last night (and it had a low priority given the difference between 68.82 degrees F and 68.83 degrees F isn't important). The other comments suggested profiling with stdio.h linked -- profiling isn't the problem -- executable size is. Simply linking stdio.h for snprint() only, balloons the executable size by 57% from 126908 bytes without to 199504 with. I'll tinker a bit with what you have a report further. \$\endgroup\$ Jan 31 at 5:17
  • \$\begingroup\$ @DavidC.Rankin I am surprised, with space as a concern, why double is used - unless it is native to the processor. IAC I'd consider using temperature as an int16_t in units of centi-degrees. \$\endgroup\$ Jan 31 at 5:23
  • \$\begingroup\$ No, the thought-of-the-moment was write generic, so I chose double, but float would be optimal given the platform -- no need to double the number of registers needed for each temperature value... The floating-point issue has to do with the way the analog-to-digital conversion is done based on a reference voltage that is sampled and scaled to obtain a temperature reading. The native result of the conversion is float so that began the experiment. While most temp sensors give 10th of a degree as well, I started with 100th of a degree knowing the last place would be basic throw-away later :) \$\endgroup\$ Jan 31 at 5:43
  • \$\begingroup\$ @DavidC.Rankin Hmmm. If prec is only a short range of possible values like [0-3], use a look-up table instead of for (int p = 0; p < prec; p++) { d *= 10.0; }. Faster and better precision for what is essentially a repetitive calculation. \$\endgroup\$ Jan 31 at 6:50
  • \$\begingroup\$ I've combined some of your thoughts (got rid of ip and fp and did it all at once), protected the bounds of s with a p > tmp check on all additions to the buffer, added rounding in the last place with fpm = (uint64_t)(d * mult + .5); and then did pretty much what you suggest in your comment above since prec will normally be less than 3, I just added a counter and in the loop converting digits, when cnt == prec I add the '.' then -- nothing fancy. I like a lot of the other additions for general use, but I was really avoiding other headers, unlinked math.h macros are fine. \$\endgroup\$ Jan 31 at 7:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.