Command line calculator in C. What is wrong?

Question

I have this code as a simple command line calculator in C.

#include <stdio.h>
#include <math.h>

int main(int argc, char* argv[]){

int num1, num2, result;
char op1;

/* convert 1st and 3st of the arguments into int */
/* assign the argv[1] to num1, assign the argv[2] to op1,
 assign the argv[3] to num2 */

if (argc == 4){
    sscanf(argv[1],"%d",&num1);
    scanf(argv[2],"%c",&op1);
    sscanf(argv[3],"%d",&num2);
}

else{   // if the input is wrong
    printf("Input Error\n");
}

switch (op1){

    case'+':
        result = num1 + num2;
        break;
    case'-':
        result = num1 - num2;
        break;
    case'x':
        result = num1 * num2;
        break;
    case'/':
        result = num1 / num2;
        break;
    case'%':
        result = num1 % num2;
        break;
    default: printf("Input Error!!!\n");
}
printf("%d\n", result);
}

After I complied the code and ran it. Some error happens.

For example, I use command: ./main 1 + 2 (Which should be 3)

The output is: Input Error!!! 0

And if I use command: ./main

The output is: Input Error Input Error!!! 0

I don't where is wrong. Please help!! Thank you so much!! I'm a completely beginner to C.

Answer 1

Some problems:

1. You used scanf instead of sscanf for parsing a character.
2. You don't check the return value of those sscanfs.
3. The switch and the final printf should be inside the if, not outside it.
4. You don't check for division by zero issues.

Here is the corrected code with some more minor modifications (untested):

#include <stdio.h>
/* #include <math.h> Unused header */

int main(int argc, char* argv[])
{
    int num1, num2, result;
    char op1;

    if (argc != 4)
    {
        fprintf(stderr, "Program requires exactly 3 arguments (excluding program name), Too %s given; Exiting...\n", argc < 4 ? "few" : "many");
        return -1;
    }

    if(sscanf(argv[1], "%d", &num1) == 1 &&
       sscanf(argv[2], "%c", &op1 ) == 1 &&
       sscanf(argv[3], "%d", &num2) == 1)
    {
        switch (op1)
        {
            case '+':
                result = num1 + num2;
                break;

            case '-':
                result = num1 - num2;
                break;

            case 'x':
                result = num1 * num2;
                break;

            case '/':
                if(num2 == 0)
                {
                    fputs("Division by zero is not possible; Exiting...", stderr);
                    return -2;
                }
                result = num1 / num2;
                break;

            case '%':
                if(num2 == 0)
                {
                    fputs("Division by zero is not possible; Exiting...", stderr);
                    return -3;
                }
                result = num1 % num2;
                break;

            default:
                fprintf(stderr, "Invalid operator '%c'; Exiting...\n", op1);
                return -4;
        }

        printf("Result = %d\n", result);
    }
    else
    {
        fputs("Failed to parse arguments successfully; Exiting...", stderr);
        return -5;
    }

    return 0;
}

Oh, and the shell has some special characters that do stuff. To avoid issues, wrap each argument with " like:

./main "1" "+" "2"

Answer 2

There is no need to sscanf() given every part of the expression is in a separate string already:

if (argc == 4){
    num1 = atoi(argv[1]);
    op1 = argv[2][0];
    num2 = atoi(argv[3]);
}