C - Strange behavior with a post-increment on isdigit() call inside while loop

Question

I have a simple program with a function to check if a C string has only a whole number on it, if there is it returns true (1) or false (0) :

#include <ctype.h>
#include <stdio.h>
#include <stdbool.h>

bool isStrWholeNumber(const unsigned char *s) {
    if (*s == '\0') return false;

    while (isdigit(*s++));

    if (*s == '\0') return true;

    return false;
}

int main()
{
    unsigned char str[] = "a";
    bool b = isStrWholeNumber(str);

    printf("%d\n", b);

    return 0;
}

The pointer increment *s++ should pass the value before increment to the isdigit function, but it seems it's passing the value after increment, so it's passing the character '\0' instead of 'a' because the function is returning true.

Changing the function to increment the pointer outside the function call, works, returning false for character 'a':

bool isStrWholeNumber(const unsigned char *s) {
    if (*s == '\0') return false;

    while (isdigit(*s)) s++;

    if (*s == '\0') return true;

    return false;
}

Why is the while (isdigit(*s++)); not working?

---

Conclusion

This is what happens when you are tired or don't sleep well, you end doing mistakes like this. The program was working correctly as you can see in answers.

After some rest, I got back to this function and got in a good result, fast and small as I wanted. I used the GCC profiler and gprof to test for performance and also checked the assembly for performance and code size. Tested with and without GCC optimization -O3.

Here is the result with comments, so you can understand it:

 bool isStrWholeNumber(const unsigned char *s) {
    if (*s) {    // Check if string is not empty
        --s;     // Decrement pointer. It will be incremented in while bellow
        while (isdigit(*++s));    // Iterate over string until a non digit character is found
        return !*s;    // Returns 1 if end of string was reached, else, return 0
    }

    return false;    // Always returns false if string is empty
}

Probably this function could be optimized even further, but I don't know how.

This function is considered by many bad code because of poor legibility, don't go around using it everywhere.

Answer 1

Program is behaving as it should. In first code, isdigit(*s++) will return 0 and s will be incremented and the statement if (*s == '\0') return true; will return true.
In second snippet, *s++ will not be evaluated and the statement return false; will return false.

Answer 2

As @haccks explains the program is behaving as expected. You are always incrementing s regardless of the outcome of isdigit. This is the reason you should always write for clarity not for brevity. This slight modification to your program makes it do what you want and also makes it much more clear:

bool isStrWholeNumber(const unsigned char *s) {
   if (*s == '\0') return false;

   while (isdigit(*s))
       ++s;

   if (*s == '\0') return true;

   return false;
}

Your original program relied on:

1. The ++ operator has precedence over the * operator so we are not accidentally increasing the content of the pointer.
2. ++ returns the value before increasing the pointer so we'll get the correct pointer value.
3. isdigit will get the dereferenced value.

With so much happening in a single line of code is it any surprise it is not behaving as you expected?

By the way, your code is a segment access violation waiting to happen if you don't get a properly terminated string.

Answer 3

The increment is performed unconditional for the first version. So, whether the test fails or not, the pointer is incremented, so it always points to the next char. If the test fails because *s is NUL ('\0'), it is still incremented by 1. The following if accesses the character past the terminator, which is undefined behaviour if the array is not large enough. It is, however, definitively unwanted as you are accessing past the end of the sting.

If you put the increment into the body of the loop as in the second version, it will only increment while the test is true. That is the wanted behaviour (as far as I see from the name of the function).

Note: you do not need to derefence the pointer to increment.