1

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.

chqrlie
  • 131,814
  • 10
  • 121
  • 189
yer
  • 21
  • 4
  • 5
    Because you explicitly read the first character `[0]` and store it in a char instead of storing the whole string. – CherryDT Apr 05 '22 at 07:42
  • [Why shouldn't I use `atoi()`?](https://stackoverflow.com/q/17710018/995714). Use `strtol` instead – phuclv Apr 05 '22 at 08:42

2 Answers2

0

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;
}
Lundin
  • 195,001
  • 40
  • 254
  • 396
  • The conversion is still somewhat *broken*: the OP defines `rowMap` as an `int`, hence `rowMap = strtol(arg1, &endptr, 10);` has implementation-defined or an implementation-defined signal is raised if the converted value is out of range. – chqrlie Apr 05 '22 at 08:28
  • @Darth-CodeX Oops, classic mistake, fixed, thanks. – Lundin Apr 05 '22 at 08:58
  • @chqrlie That's pretty much only relevant for 16 bit hosted systems... if portability to such antique systems is required, the solution is to use `stdint.h` types instead. – Lundin Apr 05 '22 at 09:02
  • Not just 16 bit hosted systems... `int` and `long` may have a different size on more modern systems. They both have 32 bits on Microsoft Windows for questionable legacy issues, but `long` has 64 bits on most 64-bit unices and macOS. – chqrlie Apr 05 '22 at 10:29
  • You advocate the use of `strtol()` as a safe replacement for `atoi()`, supposedly *broken by design*... The only problem with `atoi()` is its behavior on out of range values, but your usage of `strtol()` on systems where `long` is larger than `int` still has strange behavior on such values (for example: `2147483648` or `4294967301`). – chqrlie Apr 05 '22 at 10:30
  • @chqrlie You are kind of missing the point: "cannot be represented" is not just an integer range issue. The far more likely scenario is `atoi(argv[i])` where non-digit characters are passed to the function. See for example [CERT C ERR07-C](https://wiki.sei.cmu.edu/confluence/display/c/ERR07-C.+Prefer+functions+that+support+error+checking+over+equivalent+functions+that+don%27t) – Lundin Apr 05 '22 at 10:38
  • `atoi()` and `strtol()` have the same behavior if the string does not start with the representation of an integer: they both return `0`. This is documented and not a problem unless strict validation of input is required. `strtol()` does make it easier to detect such cases and you use that in your answer... But more cases of invalid input could be tested and reported. For simple throw away code, `atoi()` is fine and simple. The OP's problem was much more basic anyway. – chqrlie Apr 05 '22 at 14:11
  • @chqrlie Documented where, exactly? The only description is literally the one in 7.22.1: "The functions atof, atoi, atol, and atoll need not affect the value of the integer expression errno on an error. If the value of the result cannot be represented, the behavior is undefined." It may return anything or crash, since it is UB. Sure in practice many libs implement atoi like a wrapper on top of strtol, but that's another story. – Lundin Apr 05 '22 at 14:41
  • It is documented for `strtol()` and it would take a perverse implementation of `atoi()` to do anything else. The C Standard took a more restrictive definition for `atoi` than K&R in the second edition of *The C Programming Language* (Appendix B5: *`int atoi(const char *s)` converts `s` to `int`. it is equivalent to `(int)strtol(s, (char **)NULL, 10)`*). The Standard adds *Except for the behavior on error* but does not specify what exactly is to be considered an error in `strtol`: eg: `errno` is not set by `strtol` if the subject string is empty, do does it qualify as an error or a feature? – chqrlie Apr 05 '22 at 15:25
  • @chqrlie Never underestimate how many weird optimizations compilers do though. Suppose someone decides to take advantage of this UB while inlining atoi, then draw some conclusion about code generation. Like `if(!atoi("hello!"))` - aha, this is UB, we don't need to execute the `if` statement. Even though in practice atoi likely returns 0. – Lundin Apr 06 '22 at 06:14
  • This would be a good example of a *perverse compiler* :) – chqrlie Apr 06 '22 at 07:23
  • @chqrlie I prefer to call it gcc... [one example](https://stackoverflow.com/questions/52163870/bool-type-and-strict-aliasing). – Lundin Apr 06 '22 at 09:30
0

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;
    }
    [...]
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • Using `atoi` in general is bad practice, particularly so when taking command line argument inputs. We should point this out in answers so that people eventually stop using the broken parts of the standard lib. – Lundin Apr 05 '22 at 07:54
  • @Lundin: I agree that `atoi` is bad practice, but I contend that the undefined behavior in the C Standard should have been just implementation defined behavior. The lack of a proper `strtoi()` alternative or a better API without `errno` such as the one used hereabove unnecessarily complicates the learning curve for beginners. Answer amended with stricter argument validation. – chqrlie Apr 05 '22 at 08:23
  • `strtoi` actually used to exist as a non-standard extension at least back in the MS DOS days when 16 bit arithmetic matters. Nowadays, such a function mostly just makes sense in low-level embedded systems. It suppose it would be neat if `stdint.h` came with `strtoi16`, `strtoui16` and so on, so we got proper, portable types straight away for cases where it matters. – Lundin Apr 05 '22 at 08:48