CODEWITHSUNDEEP
cerrnostrtol
Rez
Rez

Reputation: 187

strtol not changing errno

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?

Upvotes: 2

Views: 2023

Answers (4)

Shiv
Shiv

Reputation: 122

After hours = strtol(localTime, &par, base); statement you have to first save the value of errno. Because after this statement you are going to call printf() statement that also set errno accordingly.

printf("errno is: %d\n", errno);

So in this statement "errno" gives the error indication for printf() not for strtol()... To do so save "errno" before calling any library function because most of the library function interact with "errno". The correct use is :

hours = strtol(localTime, &par, base);
int saved_error = errno;       // Saving the error...
printf("errno is: %d\n", saved_error);

Now check it. It will give correct output surely...And one more thing to convert this errno to some meaningful string to represent error use strerror() function as :

printf("Error is: %s\n", strerror(saved_error));

Upvotes: 0

chqrlie
chqrlie

Reputation: 144550

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.

Upvotes: 2

chux
chux

Reputation: 153338

A useful trick with sscanf() is that code can do multiple passes to detect errant input:

// HH:MM:SS
int parseTime(const char *hms, unsigned long *secs) {
  int n = 0;
  // Check for valid text
  sscanf(hms "%*[0-2]%*[0-9]:%*[0-5]%*[0-9]:%*[0-5]%*[0-9]%n", &n);
  if (n == 0) return -1; // fail

  // Scan and convert to integers
  unsigned h,m,s;
  sscanf(hms "%u:%u:%u", &h, &m, &s);
  // Range checks as needed
  if (h >= 24 || m >= 60 || s >= 60) return -1;

  *sec = (h*60 + m)*60L + s;
  return 0;
}

Upvotes: 0

R.. GitHub STOP HELPING ICE
R.. GitHub STOP HELPING ICE

Reputation: 215193

strtol is not required to produce an error code when no conversion can be performed. Instead you should use the second argument which stores the final position after conversion and compare it to the initial position.

BTW there are numerous other errors in your code that do not affect the problem you're seeing but which should also be fixed, such as incorrect use of strncpy.

Upvotes: 3

Related Questions