strtol not changing errno

Question

I'm working on a program that performs calculations given a char array that represents a time in the format HH:MM:SS. It has to parse the individual time units. Here's a cut down version of my code, just focusing on the hours:

unsigned long parseTime(const char *time)
{
    int base = 10;                    //base 10
    long hours = 60;                  //defaults to something out of range
    char localTime[BUFSIZ]            //declares a local array
    strncpy(localTime, time, BUFSIZ); //copies parameter array to local
    errno = 0;                        //sets errno to 0

    char *par;                        //pointer
    par = strchr(localTime, ':');     //parses to the nearest ':'
    localTime[par - localTime] = '\0';  //sets the ':' to null character

    hours = strtol(localTime, &par, base); //updates hours to parsed numbers in the char array
    printf("errno is: %d\n", errno);       //checks errno
    errno = 0;                             //resets errno to 0
    par++;                                 //moves pointer past the null character
}

The problem is that if the input is invalid (e.g. aa:13:13), strtol() apparently doesn't detect an error because it's not updating errno to 1, so I can't do error handling. What am I getting wrong?

Answer 1

As others have explained, strtol may not update errno in case it cannot perform any conversion. The C Standard only documents that errnor be set to ERANGE in case the converted value does not fit in a long integer.

Your code has other issues:

• Copying the string with strncpy is incorrect: in case the source string is longer than BUFSIZ, localTime will not be null terminated. Avoid strncpy, a poorly understood function that almost never fits the purpose.
• In this case, you no not need to clear the : to '\0', strtol will stop at the first non digit character. localTime[par - localTime] = '\0'; is a complicated way to write *par = '\0';

A much simpler version is this:

long parseTime(const char *time) {
    char *par;
    long hours;

    if (!isdigit((unsigned char)*time) {
        /* invalid format */
        return -1;
    }
    errno = 0;
    hours = strtol(time, &par, 10);
    if (errno != 0) {
        /* overflow */
        return -2;
    }
    /* you may want to check that hour is within a decent range... */
    if (*par != ':') {
        /* invalid format */
        return -3;
    }
    par++;
    /* now you can parse further fields... */
    return hours;
}

I changed the return type to long so you can easily check for invalid format and even determine which error from a negative return value.

For an even simpler alternative, use sscanf:

long parseTime(const char *time) {
    unsigned int hours, minutes, seconds;
    char c;

    if (sscanf(time, "%u:%u:%u%c", &hours, &minutes, &seconds, &c) != 3) {
        /* invalid format */
        return -1;
    }
    if (hours > 1000 || minutes > 59 || seconds > 59) {
        /* invalid values */
        return -2;
    }
    return hours * 3600L + minutes * 60 + seconds;
}

This approach still accepts incorrect strings such as 1: 1: 1 or 12:00000002:1. Parsing the string by hand seem the most concise and efficient solution.