C fork() causing unexpected function calls that aren't specified

Question

My first Stack Overflow post, I'm a lurker on the site but I really wanted to start asking some questions of my own!

I'm learning about the fork() system call, using the unistd.h library. I want to test out what I'm learning. I have followed a textbook for an example on a simple process creation in UNIX. I am doing this on OSX. I have been set a task to create a shell, I do not want to be given a complete answer to building a shell, hence why I'm asking about this specific system call, not the entire program.

The code runs completely fine on its own and gives the expected output.

This is the code working fine:

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

int main() {
    pid_t pid;
    pid = fork();
    if(pid == 0){
        printf("\nI'm the child\n\n");
    }
    else if(pid > 0){
        printf("\nI'm the parent, child has pid: [%d]\n", pid);
    }
    else{
        printf("ERROR");
    }
    return 0;
}

My issue is that when I put this in my shell program it is causing, from what I have found online, a fork bomb.

The function that creates this process is called with the command 'test'. When I run the function, it gives the same output as in the stand alone version however when I then move on and exit out, it seems to be calling that test function again in an infinite loop. I then can't kill the processes because my computer lags out, clearly because a bunch of processes are being created and I have to restart my computer. I've done this a few times now and it is becoming impossible to test because of the need to perform restarts every time I run it.

I have a while loop that is expecting an exit command to end. The code should be self explanatory, shown below. I know a lot of this is bad practise, I am just trying to test out these concepts, from research I believe I should be using the wait() system call but I want to understand why my code is causing this to happen.

The full program:

main.c

#include "shell.h"

int main() {
    clear();
    printf("Jack's Shell\n");
    shellLoop();
    clear();
    return 0;
}

shell.c

#include "shell.h"

void shellLoop(){
    char command[MAX_STRING_LEN];
    int exit = 1;
    while (exit != 0){
        printf("\njackdewinter$ ");
        scanf("%s", &command);
        exit = commandInterpreter(command);
    }

}

int commandInterpreter(char * cmd){
    char message[MAX_STRING_LEN];
    if(strcmp(cmd, "exit") == 0){ /* Best way to test strings (Not just characters.) */
        return 0;
    }
    else if(strcmp(cmd, "info") == 0){
        info();
        return 1;
    }
    else if(strcmp(cmd, "pwd") == 0){
        shellMessage("Call PWD");
        return 1;
    }
    else if(strcmp(cmd, "cd") == 0){
        shellMessage("Call CD");
        return 1;
    }
    else if(strcmp(cmd, "test") == 0){
        shellMessage("Test Called.");
        test();
        return 1;
    }
    else if(strcmp(cmd, "help") == 0){
        shellMessage("Call HELP");
        return 1;
    }
    else{
        shellMessage("Incorrect command entered.\nType help for assistance and a list of commands.");
        return 1;
    }
}

void info(){
    shellMessage("This shell was created by Jack Dewinter");
}

void test(){
    pid_t pid;
    pid = fork();
    if(pid == 0){
        printf("Child\n");
    }
    else if(pid > 0){
        printf("I'm the parent, child has pid %d\n", pid);
    }
    else{
        printf("ERROR");
    }
}

void shellMessage(char * message){ /* Using this for consistent shell messages. */
    printf("\n%s\n", message);
}

shell.h

#define MAX_STRING_LEN 80

#define clear() printf("\033[H\033[J") /* Terminal escape codes for clearing the console */

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

void shellLoop();
int commandInterpreter(char * cmd);
void info();
void test();
void shellMessage(char * message);

Any help would be greatly appreciated, thanks!

This isn't a fork bomb, it's just a bug. The child process should `exit()` when finished. I.e. within the `if (pid == 0)` block. Eventually that forked process should terminate. From what I see, there is no reason for the parent to care whether/when it does, but it still should, or it too will return to the command interpreter and choice menu in its process. — WhozCraig, Dec 01 '17 at 21:21
Both your child and parent return to your loop. Neither exit. So now, you have two processes. When you enter something in stdin, it goes to the child. But if child exits, the parent is still running, waiting to read from stdin. If you run test multiple times, you will have multiple instances waiting on input. I don't see a loop creating a bomb though... — HardcoreHenry, Dec 01 '17 at 21:25
There is a buffer overflow in this code. You should replace `scanf("%s", &command)` with `scanf("%80s", &command)` or use `fgets` instead. — David Conrad, Dec 01 '17 at 21:26
@DavidConrad -- I don't see this causing a buffer overflow unless someone types in a really long string. It is good practice to guard against that though. — HardcoreHenry, Dec 01 '17 at 21:32
@HardcoreHenry I hope I never have the misfortune to run any code on my computer that you wrote. — David Conrad, Dec 01 '17 at 21:35
@DavidConrad -- ?? I was just saying that the _possibility_ of a buffer overflow that you pointed out (not an _actual_ overflow as you implied) is likely not the cause of the OP's issue, and while I agree that guarding against it is good practice, I highly doubt the OP actually ran in to actual buffer overflow.... — HardcoreHenry, Dec 01 '17 at 21:41
@HardcoreHenry I never said the OP actually ran into a buffer overflow, I said this code has a problem, and it does. — David Conrad, Dec 01 '17 at 21:43
"There is a buffer overflow" implies that there is a buffer overflow... — HardcoreHenry, Dec 01 '17 at 21:44
I believe the warning when compiling is caused by what you're referring to @DavidConrad, warning: format specifies type 'char *' but the argument has type 'char (*)[80]' [-Wformat] scanf("%s", &command); — jackdewinter, Dec 01 '17 at 22:00
I changed it to how you suggested @DavidConrad and I am getting a similar error: warning: format specifies type 'char *' but the argument has type 'char (*)[80]' [-Wformat] scanf("%80s", &command); — jackdewinter, Dec 01 '17 at 22:01
From using the responses to my question I have found this post that is very similar. I'm posting it here for anyone finding this post when looking for help on something similar: https://stackoverflow.com/questions/2708477/fork-and-wait-with-two-child-processes — jackdewinter, Dec 01 '17 at 22:25

score 6 · Accepted Answer · answered Dec 01 '17 at 21:22

When you create a new process with fork, the child process continues on from the the point of the fork. So your child prints "Child", but then it returns from test. So it continues to run the code that only the parent should be running.

You don't see this in your smaller piece of code because you immediately return from main causing the child process to exit.

You need to have the child exit after it is done its part. Also, the parent should wait for the child so that you're not left with zombie processes.

void test(){
    pid_t pid;
    pid = fork();
    if(pid == 0){
        printf("Child\n");
        _exit(0);    // exit child; use _exit instead of exit 
                     //to prevent atexit handlers from being called
    }
    else if(pid > 0){
        printf("I'm the parent, child has pid %d\n", pid);
        // wait for child to finish
        wait(NULL);
    }
    else{
        printf("ERROR");
    }
}

Also, in shellLoop, rename the variable exit to something else as you're masking the library function exit.

Thank you, this resolved the issue. My computer is no longer breaking when I try to run the program, the wait(NULL) also allows execution to continue, otherwise it breaks. — jackdewinter, Dec 01 '17 at 22:05
Thanks for the tip on the exit variable as well, I wrote this up quickly whilst reading the book, I'm going to completely rewrite this, I have seen many people using a status variable for these kinds of programs. — jackdewinter, Dec 01 '17 at 22:15

score 3 · Answer 2 · answered Dec 01 '17 at 21:22

3

The child you are creating, will loop like his father, and you'll be having each loop a new process. fork : creates the exact process image.

What you can do is : 1- join the child (ie the father waits for the child to finish)

2- or just add an exit() after the printf("Child\n"); which will end the child, and keeps the father running.

answered Dec 01 '17 at 21:22

Gar

852
13
20

Thanks for this. I understand that they will both be running from the point the child is created, however the loop should not have been calling the test function again unless 'test' was entered as a command, for some reason it was looping and starting that function again. – jackdewinter Dec 01 '17 at 22:07

C fork() causing unexpected function calls that aren't specified

2 Answers2