CODEWITHSUNDEEP
c
user1234689
user1234689

Reputation:

Am I using scanf incorrectly?

Each line of input is a line with a command followed by numbers (except in the exit case).

I cannot figure out what I am doing wrong. This segment is looking for the store command and then does the action store entails:

    char command[20];
    while(strcmp(command, "exit") != 0)
    {
        /*scans for command strings inputted*/
        scanf(" %s", command);
        /* handles store command*/
        if(strcmp(command, "store") == 0)
        {   
            memory[0] = 1;
            scanf("%d %d %d %d %d", &startx, &starty, &finishx, &finishy, &number);
            for( i = startx; i < finishx; i++)
            {
                for(j = starty; j < finishy; j++)
                {
                square[i][j] = number;
                }
            }
        }
     }

Upvotes: 2

Views: 1270

Answers (1)

paxdiablo
paxdiablo

Reputation: 882596

Yes, you are using it wrongly(a). The line:

scanf(" %s", command);

has no bounds checking on the input. If someone enters more than 19 characters into your program, it will overflow the char command[20] and result in undefined behaviour.

The two major problems with scanf are:

  • using it with an unbounded %s since there's no way to control how much data is entered. My favourite saying is that scanf is to scan formatted information and there's not much less formatted than user input.
  • not checking how many items were scanned - it's possible to scan less than you expected.

If you want to do it right, see here. It uses fgets to get a line, protecting from buffer overflows and detecting problems.

Once you have the line as a string, you can safely sscanf it to your heart's content, since you know the upper bound on the length and you can always return to the start of the string to re-scan (not something easy to do with an input stream).

Other problems with your code include:

  • The initial strcmp on an uninitialised buffer. It could actually be (arbitrarily) set to exit which would cause your loop not too start. More likely is that it's not a valid C string at all, meaning strcmp will run off the end of the buffer. That won't necessarily end well.
  • No checking that all your numeric items were entered correctly. You do that by checking the return value from scanf (or sscanf should you follow my advice on using the rock-solid input function linked to here) - it should be five but entering 1 hello 2 3 4 would result in a return code of one (and all but startx unchanged).
  • No range checking on input. Since square has limited dimensions, you should check the inputs to ensure you don't write outside of the array. That's both numbers that are too large and negative numbers.

(a) Syntactically, what you have is correct. However, from a actual semantic point of view (i.e., what you mean to happen vs. what may happen), there's a hole in your code large enough to fly an Airbus A380 through :-)

Upvotes: 9

Related Questions