fork() and Execve, minix system C, core dumps

Question

I'm learning C at the moment and have tasked myself with creating a shell within a Minix virtual machine, I'm doing this by using the library functions available to me already in Minix, such as ls, cd, ect...

I've encountered a problem where after forking for a child process, I'm causing core dumps, rather than executing my commands

#include<stdio.h>
#include<sys/types.h>
#include<sys/wait.h>
#include<stdlib.h>
#include<unistd.h>
#include<string.h>

    /*Initialise variables*/
    int pid;
    char *envp[] = { NULL };
    char userInput[256];

void isParent(){
    int stat;
    waitpid(-1, &stat, 0);
}

int main(int argc, char *argv[]) {

    /*Infinite loop to cause shell to be "permenant"*/
    while(1){
        /*"*" to lead every line*/
        printf("%s","*");
        /*Get user input*/
        scanf("%s", userInput); 
        /*Leave an exit clause, to not be permenantly stuck in loop*/
        if(strcmp(userInput, "exit") == 0){
            exit(1);
        }
        /*create my child process*/
        pid = fork(); 

        /*if process is parent, wait*/
        if (pid != 0){
        isParent(); 
        }
        /*Perform function typed by the user*/
        execve(userInput, &argv[1], envp);          
    }
}

This is the code I'm working with so far, when passing /bin/ls as a parameter of my shell, I can get it to print ls, twice in one user input, however it exits the shell upon performing the act, which it shouldn't. I would like to be able to use other functions, have them print once, then return to waiting for user input.

When passing no parameters the shell will only accept "exit", no other commands. If I remove the argument clause (argv[]) from my main method, execve, or both, they throw errors, which you would expect.

I have read the documentation on all of the functions I have used and have chosen them specifically, so I'd appreciate not having to change them unless what I'm doing isn't actually possible with them.

Still learning C, so I'd appreciate smaller technical terms or easier to understand phrases. I'm not really sure if whatever my problem has been posed as a question before, but I've googled my problem in about 20 different ways and most versions of my problem have been written for c++, c# or aren't similar to my problem, from my understanding.

I'll be around for a few hours still so if I've missed any information feel free to comment and ask for clarification, information or anything else.

What do you think happens when `isParent()` returns? Without an `else`, it'll run `evecve()`, which never returns. — EOF, Nov 05 '15 at 22:38
@EOF I expect `isParent()` to be caught in an infinite loop so that my shell can continue running whilst the child is performing my functions for me, then once the child has finished doing it's thing, can return to the shell to perform another function. — Ben Bowen, Nov 05 '15 at 22:46
when check for a command line parameter, never access beyond `argv[0]` until checking `argc` to assure the command line parameter actually exists. Accessing a non-existent parameter will either cause a seg fault for access via a NULL pointer or access some random area. In either case, the result is undefined behaviour. I.E. check `argc` before accessing argv[] — user3629249, Nov 05 '15 at 22:47
because of the unused parameter `argc`, the posted code does not cleanly compile — user3629249, Nov 05 '15 at 22:48
this lline: `scanf("%s", userInput);` has a couple of problems: 1) always check the returned value (not the parameter value) to assure the operation was successful. 2) the format specifier `%s` has no size limit, so the user can easily overrun the input buffer `userInput[]`. To avoid this problem, put a max length modifier on the %s format specifier that is one less than the length of the input buffer. (remember, scanf() with %s always appends a NUL byte on the end of the buffer) In this case, suggest: `scanf("%255s", userInput); — user3629249, Nov 05 '15 at 22:53
the posted code contains some `magic` numbers. `magic` numbers make the code much harder to understand and to debug/maintain. Suggest using #define's or an enum to give the numbers meaningful names, then use those meaningful names throughout the code. In this case, the magic number 256 — user3629249, Nov 05 '15 at 22:55
in C, a successful exit returns 0, any other number is (typically) an error indication. so when the user requests an exit, by entering `quit`, the return code should be 0, not 1. Note 0 can also be reference by using the `EXIT_SUCCESS` as defined in stdlib.h — user3629249, Nov 05 '15 at 22:58
the `fork()` function can have any of three return indications: >0 means in parent ==0 means in child and <0 means an error occurred. The code is not properly handling these conditions. suggest something more like: `pid = fork(); if( pid ==0 ) { // then child execve(); perror( "exeve() failed"); exit( EXIT_FAILURE )' } else if( pid > 0 ) { then parent ... } else { // fork() failed perror( "fork() failed" ); exit( EXIT_FAILURE ); }` — user3629249, Nov 05 '15 at 23:04

score 0 · Answer 1 · answered Nov 05 '15 at 22:46

0

Change:

    /*if process is parent, wait*/
    if (pid != 0){
    isParent(); 
    }
    /*Perform function typed by the user*/
    execve(userInput, &argv[1], envp);

to:

    /*if process is parent, wait*/
    if (pid != 0){
    isParent(); 
    }
    else
    {
    /*Perform function typed by the user*/
    execve(userInput, &argv[1], envp);
    _exit(0); /* just in case */
    }

answered Nov 05 '15 at 22:46

David Schwartz

179,497
17
214
278

1

...and by "just in case", you explicitly mean "in case `execve()` fails", right? – EOF Nov 05 '15 at 22:48
I'm assuming that this logic is to stop the parent being able to access the execve function, regardless of the fact that is trapped inside the method call? @EOF – Ben Bowen Nov 05 '15 at 22:53
1

@BenBowen: You *do* realize that `waitpid()` is supposed to return eventually? – EOF Nov 05 '15 at 22:54
It's to prevent the parent from calling `execve` when it returns from `isParent` and to keep the child from running the parent's "at exit" handlers if `execve` fails. – David Schwartz Nov 05 '15 at 23:00
@EOF I did not, I'm self teaching, so I may have missed a few points here and there, I moved `waitpid()` and related code, back down into the main method, that should allow it to escape, when it needs to, right? – Ben Bowen Nov 05 '15 at 23:00
1

@BenBowen "escape" *what*? – EOF Nov 05 '15 at 23:03
1

@BenBowen: Oh, wait. Don't tell me you think that `isParent()` cannot `return`, because it doesn't end in a `return`-statement? – EOF Nov 05 '15 at 23:07
@EOF escape the wait, my understanding of wait is that it would pause until the child has finished whatever it was doing, then continue running through the code, but since the child is running tasks for the user, it would never end, meaning the shell stays open for the user until the exit it using my exit command. – Ben Bowen Nov 05 '15 at 23:15
1

@BenBowen: `waitpid()` returns when the child exits. The child exits when the program you `execve()`d terminates. Surely you've written some programs before? You'll have noticed that most useful programs eventually terminate... – EOF Nov 05 '15 at 23:18
@EOF I've written major java and some tutorial-esque c, but the idea of this shell was that it never terminates, unless I tell it to using the ` if(strcmp(xxx) ` clause in my code – Ben Bowen Nov 05 '15 at 23:22
1

@BenBowen: Well, unless you plan to recursively call your shell in your shell, *most other programs* you execute will terminate. – EOF Nov 05 '15 at 23:23
@EOF I think my understanding of what I'm doing has gotten away from me, so i'm going to rewrite the whole thing with the advice everyone has given me, and since you've taken the time to discuss this with me, thanks a bunch – Ben Bowen Nov 05 '15 at 23:28

fork() and Execve, minix system C, core dumps

1 Answers1