-4

Hi so i have this code and am really confused as to why it is not outputting invalid input, if i enter 4 / a into command argument, argc != 4 so therefore invalid input should be printed which is in the else statement? i also tried adding a separate statement but i could get that to work either. thanks

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

int main(int argc, char *argv[])
{
    float numberOne;
    float numberTwo;
    float theResult;
    char action;

    sscanf(argv[1], "%f", &numberOne);
    sscanf(argv[2], "%c", &action);
    sscanf(argv[3], "%f", &numberTwo);

    if (argc == 4) {
        switch (action) {
        case '+' :
            printf("%f \n", numberOne + numberTwo);
            break;
        case '-' :
            printf("%f \n", numberOne - numberTwo);
            break;
        case '*' :
            printf("%f \n", numberOne * numberTwo);
            break;
        case '/' :
            theResult = numberOne / numberTwo;
            if (numberTwo == 0) {
                printf("invalid output");
                break;
            } else if (theResult == -0) {
                printf("invalid input");
                break;
            }
            break;
        }
    } else {
        printf("invalid input");
    }
    return(0);
}

When I run it:

$ ./a.out 4 / a
$ 

Why doesn't it print "Invalid input"?

klutt
  • 30,332
  • 17
  • 55
  • 95
Squidly
  • 49
  • 2
  • 2
    you should check the value of `argc` *before* you start looking at values in `argv` – Chris Turner Nov 03 '17 at 14:55
  • I don't understand. How are you calling your program on the command line? You need to check the value of `argc` before your access particular members of the `argv` array. If `argc` is 1 then `argv[2]` is accessing memory outside the bounds of the array. – MFisherKDX Nov 03 '17 at 14:56
  • I'm voting to close this question as off-topic as the problem **you ask about** can't be reproduced -- you even show `argv` with 4 elements (0 - 3). But this code has another severe problem: why do you **first** access the arguments and only **later** check, whether they are actually there? This will be **undefined behavior** if too few arguments are given. –  Nov 03 '17 at 14:56
  • so if it enter "4 / a" into command arguments, argc == 3 so therefore the else from if statement prints invalid input. am i wrong? – Squidly Nov 03 '17 at 15:08

3 Answers3

2

If you call your program like

myprog 4 / a

then the sscanf(argv[3], "%f", &numberTwo); will fail and no assignment will be made to numberTwo. Because numberTwo is an uninitialized local variable, it can have any value and most likely not be 0.0. Hence your check if (numberTwo == 0) will fail.

Paul Ogilvie
  • 25,048
  • 4
  • 23
  • 41
1

argc is 4. You forget the name of the program, which is argv[0].

Bart Friederichs
  • 33,050
  • 15
  • 95
  • 195
  • i have argc is 4 and enter 3 thing into command argument – Squidly Nov 03 '17 at 14:55
  • If you provide three items on the command line, `argc` is set to 4, because it also counts the program name. See also https://stackoverflow.com/questions/3024197/what-does-int-argc-char-argv-mean – Bart Friederichs Nov 03 '17 at 14:56
  • @Squidly C arrays are 0-indexed. At index 0 of the argument list is the program name itself (by convention). –  Nov 03 '17 at 14:57
  • From the code the OP posted, I don't understand why you think the OP forgets `argv[0]` is the name of the program. Looks like he/she intentionally skips `argv[0]` when assigning intermediate variables. – MFisherKDX Nov 03 '17 at 14:59
  • @MFisherKDX, OP says: "if i enter 4 / a into command argument, argc != 4". Which isn't so. – Bart Friederichs Nov 03 '17 at 15:00
  • @MFisherKDX look at the title. –  Nov 03 '17 at 15:00
  • Hmm ... got it. I missed the *a* of "4 / a". That *a* doesn't make much sense with `%f`. – MFisherKDX Nov 03 '17 at 15:05
  • so if it enter "4 / a" into command arguments, argc == 3 so therefore the else from if statement prints invalid input. am i wrong? – Squidly Nov 03 '17 at 15:06
  • @Squidly yes, you're wrong. Are you trolling? –  Nov 03 '17 at 15:19
  • This does not solve the problem – klutt Nov 03 '17 at 16:18
  • @klutt it answers the question. We can only *guess* what actual problem OP has... –  Nov 03 '17 at 17:26
  • @FelixPalmen The question was why it doesn't print `invalid input` when called with the arguments `4 / a`. This answer has absolutely nothing to do with that. The solution is to check the result of `sscanf`. – klutt Nov 03 '17 at 17:33
  • @klutt wrong. In the code, "invalid input" is only reached for argc != 4 which OP assumed would be the case. –  Nov 03 '17 at 17:41
  • @FelixPalmen Sorry for being (very) unclear. What I meant was that was insufficient to just have a look at `argc`. This answer gives a clue, but should have been posted as a comment. – klutt Nov 03 '17 at 17:54
  • @klutt still, no. It's a direct answer to the question. OP assumes `argc` was 3, this answer explains very brief it is 4. A *great* answer would of course mention and explain all the other problems in the code, but still they weren't asked about, and given the quality of this question, I also wouldn't be eager writing a great answer. IMHO, this question should be closed anyways. –  Nov 03 '17 at 17:57
  • @FelixPalmen You're probably right that it's best to close this. I wrote an answer now before I read this comment, but I'll leave this now. – klutt Nov 03 '17 at 18:10
0

What you need to do is to check the result of sscanf. It returns the number of items it successfully filled, so:

if(!sscanf(argv[1], "%f", &numberOne)) {
    printf("invalid input");
    exit(1);
}

should do the trick.

Furthermore, you should check argc before even touching argv. Move all sscanf inside the if(argc == 4) block. Same thing goes for checking the value of numberTwo. That should be done before the division.

It would also be a good idea to split error check and calculation. Here is a complete runnable example:

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

int main(int argc, char *argv[])
{
    float numberOne;
    float numberTwo;
    float theResult;
    char action;

    if (argc != 4) {
        printf("Wrong number of arguments\n");
        exit(1);
    }

    if(!sscanf(argv[1], "%f", &numberOne)) {
        printf("Error reading argument 1\n");
        exit(1);
    }

    if(!sscanf(argv[2], "%c", &action)) {
        printf("Error reading argument 2\n");
        exit(1);
    }

    if(action != '+' && action != '-' && action != '*' && action != '/') {
        printf("%c is not a valid operation\n", action);
        exit(1);
    }

    if(!sscanf(argv[3], "%f", &numberTwo)) {
        printf("Error reading argument 3\n");
        exit(1);
    }

    if(numberTwo == 0.0 && action == '/') {
        printf("Division by zero\n");
        exit(1);
    }

    /* Now we know that everything is alright */

    switch (action) {
    case '+' :
        printf("%f \n", numberOne + numberTwo);
        break;
    case '-' :
        printf("%f \n", numberOne - numberTwo);
        break;
    case '*' :
        printf("%f \n", numberOne * numberTwo);
        break;
    case '/' :
        printf("%f \n", numberOne / numberTwo);
        break;
    }

    return(0);
}
klutt
  • 30,332
  • 17
  • 55
  • 95