CODEWITHSUNDEEP
coverflow
user13562672
user13562672

Reputation:

Detect and prevent overflow for unsigned long in C

I have some lines in my code to check whether the resulting value overflows (by comparing it to it's previous iteration), and therefore if the input value is too large. This works for some values, but not for values whose increment is so large that it not only overflows, but overflows so much that the resulting value is larger than the previous iteration. For example, it triggers for 18446744073709551616 (MAX_ULONG + 1), but not for 184467440737095516150 (MAX_ULONG * 10). How can I address this issue? The code is as follows:

unsigned long result = 0;
unsigned long overflowCheck = 0;
int power = 0;

for (int i = (strlen(input) - 1); i >= 0; i--) {
    if ((input[i] > ('0' - 1)) && (input[i] < ('9' + 1))) {
        result += (input[i] - '0') * (unsigned long)pow(iBase, power++);
    } else {
        printf("Invalid input string.");
        valid = 0;
        return -1;
    }
    if (result < overflowCheck) {
        printf("Input value too large.");
        valid = 0;
        return -1;
    }
    overflowCheck = result;
}
return result;

Upvotes: 2

Views: 421

Answers (3)

chqrlie
chqrlie

Reputation: 144715

There are multiple problems in your code:

  • you should not use pow to perform integer arithmetics: type double may have less value bits than type unsigned long (for example on 64-bit linux, double has 53 value bits and unsigned long has 64). It is simpler to multiply the current value by iBase and add the digit value for each new digit parsed.
  • it is easier to detect potential overflow before multiplying or adding the values.

Here is a modified version:

#include <errno.h>
#include <limits.h>

unsigned long atoul(const char *input, unsigned int iBase) {
    if (iBase < 2 || iBase > 36) {
        errno = EINVAL;
        return 0;
    }
    unsigned long result = 0;
    unsigned long maxval = ULONG_MAX / iBase;
    int maxdigit = ULONG_MAX % iBase;

    for (;;) {
        int c = *input++;
        int digit;
        if (c >= '0' && c <= '9') {
            digit = c - '0';
        } else
        if (c >= 'A' && c <= 'Z') {
            digit = c - 'A' + 10;
        } else
        if (c >= 'a' && c <= 'z') {
            digit = c - 'a' + 10;
        } else {
            break;
        }
        if (digit >= iBase)
            break;
        if (result > maxval || (result == maxval && digit > maxdigit) {
            /* overflow detected */
            errno = ERANGE;
            return ULONG_MAX;
        }
        result = result * iBase + digit;
    }
    return result;
}

Upvotes: 2

Zach Peltzer
Zach Peltzer

Reputation: 172

result < 0 will always return false since result is unsigned (and can never be less than 0. One way to check for overflow is to make sure pow() (as a double) is within the bounds for long. However, the real solution here is to not use pow() and keep everything as integers. If you work starting with the most significant digit, you can multiply result by the base (16 in this case) and add the new digit each time. This works because 1234 = base*(base*(base*(0 + 1) + 2) + 3) + 4

Some (incomplete) code would be:

int input_len = strlen(input);
for (int i = 0; i < input_len; i++) {
    // After finding out which digit group input[i] is in:
    result = result * iBase + (input[i] - '0');
}

Since result will only change by a factor of 16 at most, you can check for overflow by comparing with the previous result every iteration:

unsigned long previous = result;
// Add in the next digit
if (result < previous) {
    // Overflow
}

Upvotes: -1

dbush
dbush

Reputation: 223739

Suppose you want to check if x + y overflows where x and y are both unsigned long. The naive approach would be to do this:

if (ULONG_MAX < x + y)

But this will always be false because of overflow. Instead, you would do this:

if (ULONG_MAX - x < y)

This check is algebraically the same as the first attempt but avoids issues of overflow. You can do a similar check in your case:

if ((input[i] > ('0' - 1)) && (input[i] < ('9' + 1))) {
    int digit = input[i] - '0';
    if (ULONG_MAX / 10 < result) {
        printf("overflow");
        return -1;
    }
    result *= 10;
    if (ULONG_MAX - digit < result) {
        printf("overflow");
        return -1;
    }
    result += digit;
} else {
    printf("Invalid input string.");
    valid = 0;
    return -1;
}

Upvotes: 1

Related Questions