4
\$\begingroup\$

I'm writing an embedded application in C, and at one point I need to convert a float into an ASCII representation of that float, so I can send the ASCII over a serial port. The protocol the serial port is listening over doesn't like trailing '0's, so I'm writing a function that will remove all of the unnecessary '0's at the end of the ASCII string. Here is what my expected inputs and outputs look like:

#input           #output
300.000      --> 300.
300.01400    --> 300.014
300.1200     --> 300.12
300.12345678 --> 300.12345   #Only 5 decimal places of precision

Here is the function:

int float_to_str(float f, char *output, int buffer_size)
{
    int buf_len = snprintf(output, buffer_size, "%0.5f", f);

    //Strip trailing '0's from response
    bool seen_point = false;
    int real_len = 0;
    int index = 0;
    while (index < buf_len && index < buffer_size)
    {
        if (output[index] == '.')
            seen_point = true;
        if (output[index] != '0' || !seen_point)
            real_len = index + 1;
        index++;
    }

    return real_len;
}

The return value is the length of the string stuffed into output, which will never be more than buffer_size.

Is this well written and easy to understand? Am I overlooking edge-cases? How about the choice of algorithm, is there a simpler way to do this that I'm overlooking?

\$\endgroup\$
  • \$\begingroup\$ Are you okay with leaving the . alone at the end when all zeros are removed? \$\endgroup\$ – glampert Apr 18 '16 at 18:38
  • \$\begingroup\$ @glampert Yes, that is desired. The program that listens to the board watches for a . in the number to tell if it's a float or an int. \$\endgroup\$ – DJMcMayhem Apr 18 '16 at 18:40
  • \$\begingroup\$ Question: for your last input example, you say 300.123456789 should yield 300.12345. Does that mean you don't want the nearest rounded 5-digit precision value (which would be 300.12346)? Do you mean to truncate, or do you want the number rounded? \$\endgroup\$ – scottbb Apr 18 '16 at 20:09
  • 2
    \$\begingroup\$ Typical float x = 300.123456789; will result in a value of 300.123443... and that value printed with "%.5f" becomes "300.12344", not the 300.12345 posted here. \$\endgroup\$ – chux Apr 18 '16 at 21:47
2
\$\begingroup\$

My thoughts are as follows:

  1. Since you are leveraging snprintf, you have already limited the string size to buffer_size, so the additional condition in the while loop should not be needed.

  2. Since there is no validation, I'm assuming the data has been validated before and that the value will never exceed buffer_size with, which allows for the following simplification:

    int float_to_str(float f, char *output, int buffer_size)
    {
        int buf_len = snprintf(output, buffer_size, "%0.5f", f);
        if(buf_len>buffer_size) return -1 ; // updated to add validation
        //Strip trailing '0's from response
        while (buf_len>0)
        {
            if (output[buf_len-1] == '0') // is the last char a zero?
            {
                output[buf_len-1]=0;  // null terminate the string
                buf_len--;  
            }
            else break;  // if this is not a zero, we are either at the . or a non zero digit
        }
        return buf_len;  
    }
    
  3. I'm not sure your code will return the correct result if the input is 300.00014, my initial thought is your output would be 300.00.

\$\endgroup\$
  • 2
    \$\begingroup\$ snprintf returns the length the float would take up if unlimited by the buffer, see e.g. here, so your code is likely to produce some nasty segfaults). \$\endgroup\$ – Jaime Apr 18 '16 at 18:45
  • \$\begingroup\$ I should point out that your code starts at the front and iterates through the string. The code I posted starts at the end of the string and walks backwards till it finds a non zero. It would never exceed 5 iterations of the loop unless there is no . in the string. \$\endgroup\$ – Horn Apr 18 '16 at 18:47
  • \$\begingroup\$ I updated my last comment, the code should have validation added. Return error code if buf_len is larger then buffer_size because we no longer have a valid value to work with. Add the following right after the snprinf line: if(buf_len>buffer_size) return -1; \$\endgroup\$ – Horn Apr 18 '16 at 18:52
  • \$\begingroup\$ You can edit your answer to include those changes. Also, this won't compile as real_len is never declared. \$\endgroup\$ – Jaime Apr 18 '16 at 18:58
  • \$\begingroup\$ Off by 1. Suggest if(buf_len>buffer_size) return -1 ; --> if(buf_len>=buffer_size) return -1 ; > vs. >= "... the null-terminated output has been completely written if and only if the returned value is nonnegative and less than n." C11 §7.21.6.5 3 \$\endgroup\$ – chux Apr 18 '16 at 21:51
1
\$\begingroup\$

In addition to @Horn fine 1st time answer: Minor stuff:

Use size_t rather than int for array sizes as it is 1) the best type for array indexing 2) the type expected by snprintf().

// int float_to_str(float f, char *output, int buffer_size) {
int float_to_str(float f, char *output, size_t buffer_size) {
  int buf_len = snprintf(output, buffer_size, "%0.5f", f);

Pedantic code would check buf_len for insufficient buffer size and encoding errors

  if (buf_len < 0 || buf_len >= buffer_size) return -1; // error

Questionable if 300.12345678 --> "300.12345" is realistic as typical float lacks that much precision and will render "300.12344" instead.

Note that float f = 300.014; will typically result in "300.01401" for similar reasons.


"protocol ... doesn't like trailing '0's" is suspicious. Detailing the protocol needs may result in a better answer for the higher level problem.

\$\endgroup\$

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.