Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I'm trying to understand the strtod() function, and how I can handle any user input given to the function. I assume I tested for everything, however I am hoping for a review to make sure and to catch anything I missed. (I know I should separate everything into functions, however, I am just trying to understand the nature of the function, therefore I left everything in main.)

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <float.h>
#include <stdbool.h>

/*@param MAXSIZE max number of chars that are converted to a 64bit double*/
#define MAXSIZE 1077 

int main() {

    char str[MAXSIZE];
    printf("Enter a rational number: ");

    for (;;) {
        if (fgets(str, sizeof str, stdin) != NULL) {
            if (strchr(str, '\n')) {
                str[strcspn(str, "\n")] = '\0';
            }
            if (str[0] != '\0') {
                break;
            }
            printf("\nTry again: ");
        }
    }

    char* endptr = NULL;
    errno = 0;
    double number = strtod(str, &endptr);

    //passes over trailing whitespace
    for (; isspace(*endptr); ++endptr);

    if (errno == ERANGE) {
        fprintf(stderr, "Error out of range...\n");
    }
    else if (*endptr != '\0') {
        fprintf(stderr, "error could not convert: %s\n", str);
    }
    else {
        printf("Your string was converted into the rational number: %lf\n", number);
    }
    printf("Your string was: %s\n", str);
    printf("Press any key to continue...\n");
    getch();
}
share|improve this question
    
BTW, asking the user to enter a "rational" number might lead them to think they can type 1/3 here. "Decimal" may be a better choice. (not an answer, as it's tangential to the requested review) – Toby Speight 21 hours ago
    
@Toby Speight, I agree completely, thank you for pointing that out! – chris360 21 hours ago
up vote 1 down vote accepted

str[strcspn(str, "\n")] = '\0'; is an overkill. fgets reads (at most) one string, that is there is at most one '\n' in the buffer, and strchr will return its position. Consider

    char * newline = strchr(str, '\n');
    if (!newline) {
        .... // handle input too long condition
    } else if (newline == str) {
        .... // handle empty input condition
    } 
    *newline = '\0';
    break;

Notice that even newline = '\0' is redundant: '\n' is a whitespace.


Do not invent error messages. This is what strerror and perror are for. Also keep in mind that strtod may set errno to errors other than ERANGE (some implementations use EINVAL as well). Consider

    if (errno != 0) {
        perror("strtod");
    }
share|improve this answer
    
Thank you! So if errno is not zero, it is some error value and perror will return what error occured? – chris360 Oct 9 at 2:32
    
I also have another question, perror(errno) does not compile, however perror(NULL) works as expected, was that a typo or am I missing something? Another thing, can I still use my fprintf() statement? It works fine, I know i'm customizing my own error message but is there a reason I shouldn't? If yes how else could I implement it? Thank you! – chris360 Oct 9 at 3:14
    
@chris360 perror will print the standard error message. strerror will return an error message, so you may chose to print it in a more fancy manner. Also yes, there was a typo, sorry. – vnp Oct 9 at 3:14
    
Okay thank you for your help! – chris360 Oct 9 at 3:19
  1. With strtod(), if (errno == ERANGE) { is problematic.

When the input string represents a value exceeding +/- DBL_MAX, errno == ERANGE is set - no problem there.

When the input string represents a tiny non-zero value -DBL_MIN < x < DBL_MIN, errno == ERANGE might be set. It is implementation-defined.

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. C11 §7.22.1.3 10

These is no truly robust solution. The best is to not flag an error when the answer is tiny.

if (errno == ERANGE) {
  if (number >= -DBL_MIN && number <= DBL_MIN) {
    errno = 0;
  }
}
  1. Printing a double using "%f" does not well present large numbers nor tiny ones. Suggest "%e" or "%g"

  2. 1077 is not the max number of char that are converted to a 64bit double. There is no C specified upper bound. Although 1077 is certainly generous, buffer size needed to print -DBL_TRUE_MIN is 1077 + 1 and so would need 1077 + 1 (\n) + 1 (\0) to read with fgets() for IEEE binary64.

IAC, the buffer size needed is not the number of character used to print a double into a string, but the number of characters needed when reading. Depending on coding goals, this could be as small as about 400 (extras characters will not likely make a difference) or pedantically: unlimited.

  1. str[strcspn(str, "\n")] = '\0'; is sufficient.

    // if (strchr(str, '\n')) {
    //   str[strcspn(str, "\n")] = '\0';
    // }
    str[strcspn(str, "\n")] = '\0';
    
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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