'using atoi to convert command-line arguments to int, is only returning the first digit entered

I have to use command line arguments to set up a map for a snake game for my uni assignment. We were not specifically told to use atoi to help convert the command line argument from string to int, However I thought the simple nature of atoi would do the trick. On testing I discovered it is only taking the first digit.

int main(int argc, char *argv[])
{
    int isUserInput;
    char arg1, arg2, arg3;

    arg1 = argv[1][0];
    arg2 = argv[2][0];
    arg3 = argv[3][0];

    isUserInput = checkUserArg(arg1, arg2, arg3);
int checkUserArg(char arg1, char arg2, char arg3)
{
    int validation;
    int rowMap, colMap, snakeLength;

    rowMap = atoi(&arg1);
    colMap = atoi(&arg2);
    snakeLength = atoi(&arg3);

    if ((rowMap < 5) || (colMap < 5) || (snakeLength < 3))
    {
        validation = FALSE;
    }
    else if ((rowMap >= 5) || (colMap >= 5) || (snakeLength >= 3))
    {
        if (snakeLength < colMap)
        {
            validation = TRUE;
        }
        else
        {
            validation = FALSE;
        }
    }
    else
    {
        validation = FALSE;
    }
    
    return validation;
}

User has to enter 3 command line arguments (./file num1 num2 num3). I used atoi to convert the string command line arguments to int, but while testing I printed the numbers back and it won't convert the second digit only the first, e.g 1-9 works, but anything from 10 onwards only shows the first digit.

Any thoughts on why this is? Cheers.



Solution 1:[1]

There are multiple problems in your code:

  • atoi is only using the first digit because you explicitly extract the first digit and pass it as a char. The function call actually has undefined behavior as &arg1 is the address of a single char, not that of a null terminator C string.

  • checkUserArg converts the arguments using atoi which has undefined behavior if the value converted exceeds the range of type int. Using strtol is recommended and allows for finer checks.

  • checkUserArg should return the converted values to the caller via pointer arguments.

  • the second test in checkUserArg is redundant: if the first test is false, then all 3 comparisons in the second test will be true.

  • instead of TRUE and FALSE, you should use definitions from <stdbool.h>.

Here is modified version:

#include <errno.h>
#include <limits.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>

bool convertArg(const char *arg, int *vp) {
    char *p;
    long num;

    errno = 0;
    num = strtol(arg, &p, 10);
    if (errno || p == arg || *p != '\0' || num < INT_MIN || num > INT_MAX) {
        *vp = 0;
        return false;
    } else {
        *vp = (int)num;
        return true;
    }
}

int main(int argc, char *argv[]) {
    int rowMap, colMap, snakeLength;

    if (argc != 4) {
        fprintf(stderr, "program needs 3 arguments\n");
        return 1;
    }
    if (!converArg(argv[1], &rowMap) || rowMap < 5) {
        fprintf(stderr, "invalid rowMap argument\n");
        return 1;
    }
    if (!converArg(argv[2], &colMap) || colMap < 5) {
        fprintf(stderr, "invalid colMap argument\n");
        return 1;
    }
    if (!converArg(argv[3], &snakeLength) || snakeLength < 3 || snakeLength >= colMap) {
        fprintf(stderr, "invalid snakeLength argument\n");
        return 1;
    }
    [...]
}

Solution 2:[2]

A single character is not a string. A string is an array of characters with null termination at the end.

You should do something like this instead:

bool checkUserArg (const char* arg1, const char* arg2, const char* arg3);

const since we shouldn't modify the args. Now this function can call atoi using the parameters directly.

However atoi is a broken function by design that should never be used, because it does not have well-defined error handling. Using it directly on argv is dangerous. You should always use strtol instead of atoi, it's a safer and more powerful function.

Example:

#include <stdlib.h>
#include <stdbool.h>

int main (int argc, char *argv[])
{
    if(argc != 4)
    {
      // error handling!
    }

    if(checkUserArg(argv[1], argv[2], argv[3]) == false)
    {
      /* error handling */
    }  
...

bool checkUserArg (const char* arg1, const char* arg2, const char* arg3)
{
   const char* endptr;
   ...
   rowMap = strtol(arg1, &endptr, 10); // 10 for decimal base
   if(endptr == arg1) // if these compare equal, the conversion failed
   {
     return false;
   }
   ...

   return true;
}

Sources

This article follows the attribution requirements of Stack Overflow and is licensed under CC BY-SA 3.0.

Source: Stack Overflow

Solution Source
Solution 1
Solution 2