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 :-)