3
\$\begingroup\$

The following program converts a string to a double, and checks for errors. It uses the strtod() function to do the conversion, and follows the example given for the strtol() function from the man page to do the error checking.

As an example, the program is executed as follows:

$ ./a.out 123.45

I would really appreciate any comments regarding the code, specifically whether there are any issues to be address or whether the code can be made more efficient or improved in any way.

#include <stdlib.h>
#include <float.h>
#include <stdio.h>
#include <errno.h>

int main(int argc, char *argv[])
{
    char *endptr, *str;
    double val;

    if (argc < 2) {
        fprintf(stderr, "Usage: %s str\n", argv[0]);
        exit(EXIT_FAILURE);
    }

    str = argv[1];

    errno = 0;    /* To distinguish success/failure after call */
    val = strtod(str, &endptr);

    /* Check for various possible errors */

    if ((errno == ERANGE && (val == DBL_MAX || val == -DBL_MAX))
            || (errno != 0 && val == 0.0)) {
        perror("strtod");
        exit(EXIT_FAILURE);
    }

    if (endptr == str) {
        fprintf(stderr, "No digits were found\n");
        exit(EXIT_FAILURE);
    }

    /* If we got here, strtod() successfully parsed a number */

    printf("strtod() returned %f\n", val);

    if (*endptr != '\0')        /* Not necessarily an error... */
        printf("Further characters after number: %s\n", endptr);

    exit(EXIT_SUCCESS);
}
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

Good use of errno, but can be improved.

The below is decent code for detecting overflow, yet would benefit with improvements.

On overflow, the return value is HUGE_VAL, not necessarily DBL_MAX. HUGE_VAL may be a large finite or an infinity.

if ((errno == ERANGE && (val == DBL_MAX || val == -DBL_MAX)) || ...) {
    perror("strtod");
    exit(EXIT_FAILURE);
}
// replace with 
if ((errno == ERANGE && (val == HUGE_VAL || val == -HUGE_VAL)) || ...) {

Underflow may also raise ERANGE. Look at the code that attempts to discern that and review the spec.

if (...  || (errno != 0 && val == 0.0)) {
    perror("strtod");
    exit(EXIT_FAILURE);
}

If the result underflows ..., the functions return a value whose magnitude is no greater than the smallest normalized positive number in the return type; whether errno acquires the value ERANGE is implementation-defined.

OP's code incorrectly assumes 0.0 on underflow.

With so many options with 1) if errno is set and 2) what value is return, portable code nneds to fix a mess.

Portable code would use the following, yet that gives up useful conversions by some of values in the sub-normal range DBL_TRUE_MIN to DBL_MIN.

if (fabs(val) < DBL_MIN) {
  if (val > 0.0) val = DBL_MIN;
  else if (val < 0.0) val = -DBL_MIN;
  // val is now, DBL_MIN, -DBL_MIN, 0.0, or -0.0

  // pedantic code would also reset errno = 0 if it changed due to `strtod()`
}

A somewhat less portable handing, yet retains sub-normal values - just accept any wee value.

if (fabs(val) < DBL_MIN) {
  // reset errno = 0 if it changed due to `strtod()`
}

No conversion

Good test for no conversion, yet some strtod() set errno in the no conversion case. To simplify errno processing in the previous block, I'd recommend to do the endptr == str before looking at errno.

Use exponential notation for output

"%f" may print all values of magnitude less than 0.0000005 as 0.000000. This is not informative. With large values like DBL_MAX, perhaps hundreds of digits are printed of which past the first 20 or so are rarely of interest.

Instead:

#include <float.h>

printf("strtod() returned %.*g\n", DBL_DECIMAL_DIG, DBL_val);
// or
printf("strtod() returned %.*e\n", DBL_DECIMAL_DIG - 1, val);

Fancier code would assess the length of the argv[1] string for exponential notation and number of digits and then print out in a like-wise fashion.

Tolerate trailing white-space

OP code's alerts with input like "123 ", but not with " 123". Suggest silence on trailing white-space.

// add
while (issapce((unsigned char) *endptr)) {
  endptr++;
}

if (*endptr != '\0')        /* Not necessarily an error... */
    printf("Further characters after number: %s\n", endptr);

Such non-numeric output may be best done on stderr rather than stdout.


Suggested alternative.

Useful as a basis for some ideas.

// Return status for my_strtod().
// Higher values are more problematic.
typedef enum {
  my_strtod_OK,
  my_strtod_Underflow,
  my_strtod_Overflow,
  my_strtod_ExtraJunk,
  my_strtod_NoConvertableText,
  my_strtod_N
} my_strtod_T;

// Convert a pointer to a string to double and saves its value in *dest.
// Return conversion status.
//
// `errno` is temporarily cleared by this function.  Its value on return:
//   1) Should strtod(*s, ...) set errno, then that is its value.
//   2) Otherwise errno is restored to its original value.
my_strtod_T my_strtod(double *dest, const char *s) {
  char *endptr;

  int errno_original = errno;
  errno = 0;
  *dest = strtod(s, &endptr);
  int errno_my_strtod = errno;
  if (errno == 0) {
    errno = errno_original;
  }

  if (s == endptr) {
    return my_strtod_NoConvertableText;
  }

  while (isspace((unsigned char ) *endptr)) {
    endptr++;
  }
  if (*endptr) {
    return my_strtod_ExtraJunk;
  }

  if (errno_my_strtod == ERANGE && fabs(*dest) == HUGE_VAL) {
    return my_strtod_Overflow;
  }

  if (errno_my_strtod == ERANGE && fabs(*dest) <= DBL_MIN) {
    return my_strtod_Underflow;
  }

  // Note: at this point, errno may be set
  return my_strtod_OK;
}

Selected C specs concerning errno:

/*
 * errno ...  has type int ... the value of which is set to a positive error 
 * number by several library functions.  C11dr §7.5 2 
 * 
 * errno ... is never set to zero by any library function. The value of errno
 * may be set to nonzero by a library function call whether or not there is an 
 * error, provided the use of errno is not documented in the description of 
 * the function ... C11dr §7.5 3 
 * 
 * Of course, a library function can save the value of errno on entry and 
 * then set it to zero, as long as the original value is restored if errno’s 
 * value is still zero just before the return.  C11dr footnote 202
 */
\$\endgroup\$
12
  • \$\begingroup\$ Thank you very much for reviewing my code. Those are all great suggestions. I really appreciate it. Cheers! \$\endgroup\$
    – Domenic
    Commented Jul 19, 2018 at 22:20
  • \$\begingroup\$ Oh wow, I definitely like your suggested alternative code, where my_strtod returns the status for each of the possibilities. When testing for underflow, though, won't the test fail in some cases since errno may not necessary be set to ERANGE in some implementations, as you indicated earlier? \$\endgroup\$
    – Domenic
    Commented Jul 19, 2018 at 23:31
  • \$\begingroup\$ @Domenic won't the test fail --> depends on what is "failure" and the coding goal in this case. In practice, conversion to a |value| less than DBL_MIN is simply a tolerable non-failure result - regardless of errno. Portable code could use if (fabs(val) < DBL_MIN) { if (val > 0.0) val = DBL_MIN; else if (val < 0.0) val = -DBL_MIN; return my_strtod_Underflow; } \$\endgroup\$
    – chux
    Commented Jul 20, 2018 at 1:05
  • \$\begingroup\$ @Domenic Too be clear, in the cases of fabs(y) <= DBL_MIN, there is too much left to implementation defined behavior for super portable code. This all gets very messy once one considers that strod() is subject to different results based on rounding modes (5 of them). Do the best you can and document your expectations. \$\endgroup\$
    – chux
    Commented Jul 20, 2018 at 1:08
  • 1
    \$\begingroup\$ I've now asked about this over on SO. \$\endgroup\$ Commented Jul 20, 2018 at 13:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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