Thread: Did I check for overflow or underflow properly?

  1. #1
    Registered User
    Join Date
    May 2015
    Posts
    228

    Did I check for overflow or underflow properly?

    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 :-)

  2. #2
    Registered User
    Join Date
    Jun 2015
    Posts
    1,643
    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.

  3. #3
    Registered User
    Join Date
    May 2015
    Posts
    228
    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.
    Last edited by deathslice; 03-17-2016 at 12:23 PM.

  4. #4
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    37,999
    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.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  5. #5
    Registered User
    Join Date
    May 2015
    Posts
    228
    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.
    Last edited by deathslice; 03-17-2016 at 12:54 PM.

  6. #6
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    37,999
    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
    }
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  7. #7
    Registered User
    Join Date
    May 2015
    Posts
    228
    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;
    }
    Last edited by deathslice; 03-17-2016 at 02:23 PM.

  8. #8
    Registered User
    Join Date
    May 2015
    Posts
    228
    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.
    Last edited by deathslice; 03-17-2016 at 02:25 PM.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 22
    Last Post: 09-12-2015, 01:29 PM
  2. How do I check what causes a stack overflow?
    By MutantJohn in forum C++ Programming
    Replies: 11
    Last Post: 09-24-2013, 05:00 PM
  3. Stack underflow
    By homer_3 in forum C Programming
    Replies: 5
    Last Post: 09-02-2009, 01:28 PM
  4. handling overflow, underflow etc
    By broli86 in forum C Programming
    Replies: 8
    Last Post: 07-03-2008, 07:10 AM
  5. Overflow check
    By tezcatlipooca in forum C Programming
    Replies: 5
    Last Post: 12-23-2006, 12:04 PM