Your basic problem is that you haven't allocated memory for your string. In C, you are responsible for all memory management. If you declare variables on the stack, this is easy. With pointers, it's a little more difficult. Since you have the line char* str = NULL
, when you attempt to scanf
into it, you write bytes to NULL
, which is illegal. What the %s
specifier does is write into what str
points to; it can't change str
, as parameters are passed by value. This is why you have to pass &acct
instead of just acct
.
So how do you fix it? You need to provide memory where the read-in string can live. Something like char str[5] = ""
. This makes str
a five-element character array, big enough to hold "exit" and its terminating zero byte. (Arrays decay into pointers at the slightest provocation, so we're fine on that front.) However, this is dangerous. If the user enters the string malicious
, you're going to write "malic"
into str
and the bytes for "icious\0"
into whatever comes after that at memory. This is a buffer overflow, and is a classic bug. The simplest way to fix it here is to require the user to enter a command of at most N letters, where N is the longest command you have; in this case, N = 4. Then you can tell scanf
to read in at most four characters: scanf("%4s %u %i", cmd, &acct, &amt)
. The %4s
says "read in at most four characters", so you can't screw up other memory. However, note that if the user enters malformed 3 4
, you won't be able to find the 3 and the 4, since you'll be looking at ormed
.
The reason you could do scanf("%s %u %i", &cmd, &acct, &amount)
is that C is not type-safe. When you gave it &cmd
, you gave it a char**
; however, it was happy to treat it as a char*
. Thus, it wrote bytes over cmd
, so if you passed in the string exit
, cmd
might (if it were four bytes wide and had the appropriate endianness) be equal to 0x65786974
(0x65 = e
, 0x78 = x
, 0x69 = i
, 0x74 = t
). And then the zero byte, or any other bytes you passed in, you would start to write over random memory. If you change it at strcmp
too, however, it will also treat the value of str
as a string, and everything will be consistent. As for why return 0;
fails but exit(0)
works, I'm not sure, but I have a guess: you may have been writing over the return address of main
. That's stored on the stack too, and if it happens to come after cmd
in the stack layout, then you might be zeroing it or scribbling on it. Now, exit
must do its cleanup manually, jumping to the right locations, etc. However, if (as I think is the case, although I'm not sure) main
behaves like any other function, its return
jumps to the space on the stack stored as the return address (which is probably a cleanup routine of some sort). However, since you've scribbled over that, you get an abort.
Now, there are a couple of other small improvements you could make. First, since you're treating done
as a boolean, you ought to loop while (!done) { ... }
. Second, the current setup requires you to write exit 1 1
to exit the program, even though the 1 1
bit shouldn't be necessary. Third, you should check to see if you have successfully read all three arguments, so you don't get errors/inconsistencies; for instance, if you don't fix this, then the input
deb 1 2
deb 3 a
Calls debit(1,2)
and debit(3,2)
, while still leaving the a
in the input to trip you up. Finally, you should exit cleanly on EOF, rather than looping forever doing the last thing you did. If we put this together, we get the following code:
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
void credit(unsigned int acct, int amount);
void debit(unsigned int acct, int amount);
void service_fee(unsigned int acct, int amount);
int main() {
char cmd[5] = "";
unsigned int acct = 0;
int amount = 0;
int done = 0;
while (!done) {
if (feof(stdin)) {
done = 1;
} else {
if (scanf("%4s", cmd, &acct) != 1) {
fprintf(stderr, "Could not read the command!\n");
scanf(" %*s "); /* Get rid of the rest of the line */
continue;
}
if (strcmp(cmd, "exit") == 0) {
done = 1;
} else {
if (scanf(" %u %i", &acct, &amount) != 2) {
fprintf(stderr, "Could not read the arguments!\n");
scanf(" %*s "); /* Get rid of the rest of the line */
continue;
}
if ((strcmp(cmd, "dep") == 0) || (strcmp(cmd, "deb") == 0))
debit(acct, amount);
else if ((strcmp(cmd, "wd") == 0) || (strcmp(cmd, "cred") == 0))
credit(acct, amount);
else if (strcmp(cmd, "fee") == 0)
service_fee(acct, amount);
else
fprintf(stderr, "Invalid input!\n");
}
}
/* Cleanup code ... */
}
return 0;
}
/* Dummy function bodies */
void credit(unsigned int acct, int amount) {
printf("credit(%u, %d)\n", acct, amount);
}
void debit(unsigned int acct, int amount) {
printf("debit(%u, %d)\n", acct, amount);
}
void service_fee(unsigned int acct, int amount) {
printf("service_fee(%u, %d)\n", acct, amount);
}
Note that if there is no "cleanup code", you can replace all your uses of done
with break
and remove the declaration of done
, giving the nicer loop
while (1) {
if (feof(stdin)) break;
if (scanf("%4s", cmd, &acct) != 1) {
fprintf(stderr, "Could not read the command!\n");
scanf(" %*s "); /* Get rid of the rest of the line */
continue;
}
if (strcmp(cmd, "exit") == 0) break;
if (scanf(" %u %i", &acct, &amount) != 2) {
fprintf(stderr, "Could not read the arguments!\n");
scanf(" %*s "); /* Get rid of the rest of the line */
continue;
}
if ((strcmp(cmd, "dep") == 0) || (strcmp(cmd, "deb") == 0))
debit(acct, amount);
else if ((strcmp(cmd, "wd") == 0) || (strcmp(cmd, "cred") == 0))
credit(acct, amount);
else if (strcmp(cmd, "fee") == 0)
service_fee(acct, amount);
else
fprintf(stderr, "Invalid input!\n");
}