Did I check for overflow or underflow properly?

**deathslice** · 03-17-2016

I'm testing a segment of my program for underflow and overflow. The input is coming as a command line argument. The input that is coming the command line is the number of entries to process. The number zero here means to process every entry from the file.

Here is my code with only relevant information.

Code:

int main(int argc, char *argv[])
{
   int numOfEntries, isValidEntries;
   isValidEntries = inputValidation(argv[1], &numOfEntries);


  if(!isValidEntries || numOfEntries < 0)
  {
      puts("\nThe number of entries that you entered is either too big or too small. ");
      puts("Please execute the program again.");
  }
   
   return EXIT_SUCCESS;
}

int inputValidation(char *str, int *value)
{
    char *endPtr;


    if(str[0] == '\0')
    {
       return FALSE;
    }


    errno = FALSE;


   *value = (int)strtol(str, &endPtr, BASE);


   if((errno == ERANGE && ((*value) == INT_MAX || (*value) == INT_MIN)) || (errno != FALSE && (*value) == FALSE))
   {
       return FALSE;
   }
   else if(endPtr == str || *endPtr != '\0')
   {
       return FALSE;
   }


   return TRUE;
}

In essence, if we have an overflow or an underflow the input is invalid. Else if it is not an underflow but a negative number, The input is invalid. Is there anything you see here that I forgot to check or did improperly? Cheers :-)

**algorism** · 03-17-2016

This is wrong:

Code:

errno = FALSE;

errno is not a boolean variable. Using FALSE to set it because it coincidentally contains the value you want is bad practice.

In general you've completely screwed this up by chopping off the return value to an int right away. You need to receive it as a long int, check for out-of-range values there, and then, when you know it's in range, transfer it to the int.

**deathslice** · 03-17-2016

So something like this:

Code:

int isValidInteger(char *str)
{
    char *endPtr;


    if(str[0] == '\0')
    {
       return FALSE;
    }


    errno = 0;


   long integer = strtol(str, &endPtr, BASE);


   if((errno == ERANGE && (integer == LONG_MAX || integer == LONG_MIN)) || (errno != 0 && integer == 0))
   {
       return FALSE;
   }
   else if(endPtr == str || *endPtr != '\0')
   {
       return FALSE;
   }


   return (int)integer;
}

I took away the int pointer because I was able to simplify the function and did not need it in the end.

**Salem** · 03-17-2016

Well if LONG_MAX is greater than INT_MAX (for example), you need further checks to make sure the cast isn't erasing bits of interest.

**deathslice** · 03-17-2016

Then can't we just check if it is equaled to INT MAX or INT MIN then truncate the long to an int?

Code:

long value = strtol(str, &endPtr, BASE);


   if((errno == ERANGE && (value == INT_MAX || value == INT_MIN)) || (errno != 0 && value == 0))
   {
       return FALSE;
   }
   else if(endPtr == str || *endPtr != '\0')
   {
       return FALSE;
   }

return (int)value;

Remember that this value is going to be returned to an int variable in main and that variable can only hold 2^31 bits.

**Salem** · 03-17-2016

No.

In fact, if errno is ERANGE, checking for LONG_MAX or LONG_MIN seems redundant.

If strol() returns success, then you need the additional checks of

Code:

if ( value > INT_MAX || value < INT_MIN ) {
  errno = ERANGE;  // still out of range of an int
  return 0;
} else {
  return (int)value;  // cast is now OK
}

**deathslice** · 03-17-2016

You are saying it is redundant because ERANGE, if there is an overflow or underflow, gets assigned LONG_MAX or LONG_MIN(in this case, we are just checking if the value is not longer than INT_MAX or INT_MIN). If so that makes sense. Don't forget that we also have to check if the string is a valid integer and so we need this check.

Code:

if(*endPtr != '\0')
{
  return NOTVALIDINTEGER;
}

**deathslice** · 03-17-2016

You are saying it is redundant because ERANGE, if there is an overflow or underflow, gets assigned LONG_MAX or LONG_MIN(in this case, we are just checking if the value is not longer than INT_MAX or INT_MIN). If so that makes sense. Don't forget that we also have to check if the string is a valid integer and so also we need this check.

Code:

if(*endPtr != '\0')
{
  return NOTVALIDINTEGER;
}

In essence it should look like this:

Code:

  char *endPtr;

  if(str[0] == '\0')
  {
     return NOTVALIDINTEGER;
  }

  errno = 0;

  long value = strtol(str, &endPtr, BASE);
   
  if(value > INT_MAX || value < INT_MIN ) 
  {
      errno = ERANGE;  // still out of range of an int
      return NOTVALIDINTEGER;
  }
  else if(*endPtr != '\0')
  {
      return NOTVALIDINTEGER;
  }
  else 
      return (int)value;  // cast is now OK
}

Edit: oops double post by mistake.

Thread: Did I check for overflow or underflow properly?

Thread Tools

Search Thread

Display

Did I check for overflow or underflow properly?

Similar Threads

Coding a check for array overflow? or how to have no input limit.

How do I check what causes a stack overflow?

Stack underflow

handling overflow, underflow etc

Overflow check