Thread: Checking if the arguments passed to the main function are numbers is not working

I'm trying to write a program that needs to be passed 3 numbers as arguments by means of the CLI. This is the code:

Code:

#include <ctype.h>
#include <stdio.h>
#include <stdlib.h>

int main(int argc, char *argv[]) {
    // We first check whether the user is passing 3 arguments (without including the name of the program itself)
    if (argc < 4) {
        fprintf(stderr, "You must type 3 arguments.\n");
        exit(EXIT_FAILURE);
    }

    // If 3 arguments were passed, we must check that every argument is a number
    int argument;
    for (int i = 1; i <= 3; i++) {
        argument = atoi(argv[i]);
        if (! isdigit(argument)) {
            fprintf(stderr, "Arguments must be numbers.\n");
            exit(EXIT_FAILURE);
        }
    }

    return EXIT_SUCCESS;
}

So, if I ran:

• ./program 1 2: It should complaint that the user must pass 3 arguments.
• ./program 1 2 a or ./program 1 2 "string": It should complaint that not every argument is a number.
• ./program 1 2 3: The program should run successfully.

However, the check isn't done correctly when I pass three numbers:

Code:

$ ./program 1 2 3
Arguments must be numbers.

Why is the check not working properly?

Thanks in advance.

I would even check out strtol, which gives you a pointer to the first "bad" character in the input string. If that pointer points to the first character of the string, then the string does not contain a number; otherwise, if it points to the null terminator, then the whole string contains a number.

Originally Posted by laserlight

You have a right idea with isdigit, but isdigit checks if a character (converted to int) is a digit. So you need to loop over the characters of argv[i] and check them with isdigit. atoi is a wrong approach.

Okay, now it's working properly.

Code:

#include <ctype.h>
#include <stdio.h>
#include <stdlib.h>

int main(int argc, char *argv[]) {
    // We first check whether the user is passing 3 arguments (without including the name of the program itself)
    if (argc < 4) {
        fprintf(stderr, "You must type 3 arguments.\n");
        exit(EXIT_FAILURE);
    }

    // If 3 arguments were passed, we must check that every argument is a number
    for (int i = 1; i <= 3; i++) {
        if (! isdigit(*argv[i])) {
            fprintf(stderr, "Arguments must be numbers.\n");
            exit(EXIT_FAILURE);
        }
    }

    return EXIT_SUCCESS;
}

Originally Posted by christop

I would even check out strtol, which gives you a pointer to the first "bad" character in the input string. If that pointer points to the first character of the string, then the string does not contain a number; otherwise, if it points to the null terminator, then the whole string contains a number.

Although laserlight stated that atoi is a wrong approach in this case, I will definitively check this as an alternative to atoi if I ever have trouble with it again.

Thank you all for your help

Originally Posted by rstanley

You are only checking the first char of each argument. "strtol()" will do the error checking for you when you attempt to convert the string into a number.

You are right, it's only checking the first character and this is not what I want. strtol will also return 0 if no conversion could be performed, so I don't need to check if the numbers are valid and I can store them in an array:

Code:

long numbers[3];
    for (int i = 0; i < 3; i++) {
        numbers[i] = strtol(argv[i + 1], &argv[i + 1], 0);
    }

Thank you for your help, rstanley. As for the reference manual, I'm using an online tutorial, and if I need to look for further information, I usually check The Open Group Base Specifications Issue 7, 2018 edition
The tutorial doesn't cover every function, that's why the strtol function was unknown to me. So, thanks for linking that manual, I will take a look at it.

By using &argv[i + 1] for strtol's endptr parameter, you lose the original pointer to your argument, and you can't tell if conversion succeeded or not. You might as well just use atoi and assume an argument was 0 if atoi returns 0.

What I would do is something more like this:

Code:

for (int i = 0; i < 3; ++i) {
   char *endptr;
   numbers[i] = strtol(argv[i + 1], &endptr, 10);
   if (endptr == argv[i + 1] || endptr[0] != '\0') {
      // error: argument is not numeric
   }
}

I also passed in 10 for the base parameter to ensure that all numbers are interpreted as decimal. If you want to let the user use either "10" (decimal), "0xA" (hexadecimal), or "012" (octal) for the value 10, for example, then pass in 0 as you're doing. It'll take a number in any of those bases.

Originally Posted by christop

By using &argv[i + 1] for strtol's endptr parameter, you lose the original pointer to your argument, and you can't tell if conversion succeeded or not. You might as well just use atoi and assume an argument was 0 if atoi returns 0.

What I would do is something more like this:

Code:

for (int i = 0; i < 3; ++i) {
   char *endptr;
   numbers[i] = strtol(argv[i + 1], &endptr, 10);
   if (endptr == argv[i + 1] || endptr[0] != '\0') {
      // error: argument is not numeric
   }
}

I also passed in 10 for the base parameter to ensure that all numbers are interpreted as decimal. If you want to let the user use either "10" (decimal), "0xA" (hexadecimal), or "012" (octal) for the value 10, for example, then pass in 0 as you're doing. It'll take a number in any of those bases.

Ah, I understand. Except why you check if endptr[0] != '\0', is it in case the user passes as an argument a null terminator?

I have another question: if I checked if every character of every string that were passed as arguments were a number, like this:

Code:

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

void CheckNumberOfArguments(const int NumberOfArguments) {
    if (NumberOfArguments < 4) {
        fprintf(stderr, "You must pass at least 3 arguments.\n");
        exit(EXIT_FAILURE);
    }
}

bool IsANumber(const char character) {
    bool AllowedCharacter;
    switch (character) {
        case '1':
        case '2':
        case '3':
        case '4':
        case '5':
        case '6':
        case '7':
        case '8':
        case '9':
        case '0':
            AllowedCharacter = true;
            break;
        default:
            AllowedCharacter = false;
            break;
    }
    return AllowedCharacter;
}

bool ValidString(const char *string) {
    bool valid = true;
    int i = 0;
    while (string[i] != '\0') {
        if (! IsANumber(string[i])) {
            valid = false;
            break;
        }
        i++;
    }
    return valid;
}

int main(int argc, char *argv[]) {
    CheckNumberOfArguments(argc);
    for (int i = 1; i < argc; i++) {
        if (! ValidString(argv[i])) {
            fprintf(stderr, "Arguments must be numbers.\n");
            return EXIT_FAILURE;
        }
    }
    printf("Everything's correct.\n");
    return EXIT_SUCCESS;
}

Would this be perceived as a correct solution?