C: using the system() command

Question

I'm writing a program that acts as a simple shell. Users call the program from the command line and are prompted to enter commands that are sent to the OS for completion. It should run until the user enters "done", at which point the program should break. I'm running into a problem with entering done - the program quits as it should, but prints

sh: -c: line 0: syntax error near unexpected token `done'
sh: -c: line 0: `done'

to the terminal before finishing execution. Here's the code I've written that applies:

char isDone[] = "done\n"; //fgets stores the new line character from the command line

do {
    printf(">>"); //print the prompt each time through the loop
    fgets(command, 50, stdin); //get a line from the terminal
    system(command);
} while (strcmp(command, isDone) != 0); //break the loop if the user types in done

I think the error has to do with the fact "done" is not a valid UNIX command, but I'm not sure how to deal with this error. I tried solving this problem with the following fix:

if(system(command) == -1){
    printf("The OS doesn't recognize this command");
}
else
    system(command);

But that didn't solve the problem or printing the errors to the screen, and created a second problem of printing the commands/errors twice - once in the if conditional block, and once in the else block. How can I solve this problem?

EDIT This is a homework question the requires using a do-while. Is there a solution that uses a do-while?

Answer 1

The do...while construct executes its body before the loop condition is checked. So by the time the loop "realizes" that the user entered done, it has already tried to execute that input as a command inside the loop body.

The clearest way to fix this is to use break:

while (1)
{
    fgets(command, 50, stdin);
    if (!strcmp(command, isDone)) break;
    system(command);
}

The reason to structure it this way is that each iteration consists of both actions that should be done before the condition (reading in the user input) and actions that should be done after the condition (executing the command with system()). Because of this, neither a do...while or simple while will allow you to structure your code intuitively. The break keyword gives you a way to put the loop's termination condition in the middle of the loop body.

Answer 2

The order of execution is:

fgets(command, 50, stdin); //get a line from the terminal
system(command);
strcmp(command, isDone) != 0

So the line "done" is read, sent to system (which tries to execute it as a shell command, printing an error) and only then it checks it.

You can try something like that:

for(;;){
    printf(">>"); //print the prompt each time through the loop
    fgets(command, 50, stdin); //get a line from the terminal
    if(!strcmp(command, isDone)) break; //break the loop
    system(command);
}

Edit: if you want to keep the do-while:

printf(">>"); //print the prompt each time through the loop
fgets(command, 50, stdin); //get a line from the terminal
do {
    system(command);
    printf(">>"); //print the prompt each time through the loop
    fgets(command, 50, stdin); //get a line from the terminal
} while (strcmp(command, isDone) != 0); //break the loop if the user types in done

But the break version is clearly more readable.

Answer 3

After the fgets(), do the system() call inside an if statement:

if ( strcmp( isDone, command) != 0 ) {
    system( command );
}