Kyle Gagnon
Kyle Gagnon

Reputation: 57

Linked List in C gives Segmentation Fault

I've been working on implementing a command line prompt in C for linux. Basically it executes commands using system calls. All of that works so far but the next part is implementing the history command so I would typically be using a linked list command.

I get the concept of linked list and I know them but I'm hopeless at C. I can't seem to wrap my tiny pea brain around it. So each node structure will contain the command (which is an array of strings) and then the pointer to the next node.

Where am I going wrong?

Node structure definition

struct Node {
    char** storedCommand;
    struct Node* next;
};

Global Head definition

struct Node* head = NULL;

Rest of the methods

struct Node* createNode(char** command) {
    struct Node* returnNode = malloc(sizeof(struct Node));
    returnNode->storedCommand = command;
    returnNode->next = NULL;
    return returnNode;
}

void addNode(char** command) {
    struct Node* tempNode = createNode(command);

    if(head == NULL) {
        head = malloc(sizeof(struct Node));
        head = tempNode;
    } else {
        struct Node* pointerNode = head;

        while(pointerNode->next != NULL) {
            pointerNode = pointerNode->next;
        }

        pointerNode->next = tempNode;
    }
}

void traverseList() {
    struct Node* p = (struct Node*)malloc(sizeof(struct Node));
    p = head;
    printf("%s", head->storedCommand[0]); // This line fails but why; I thought I have already defined head
    while(p != NULL) {
        printf("%s", p->storedCommand[0]);
        p = p->next;
    }
}

So I put a comment where it is faulting at but I really don't know why.

Here is my main

int main() {

    while(TRUE) {
        printPrompt();
        cmdLine = readCommandLine();
        cmd = parseCommandLine(cmdLine);

        if(strcmp(cmd[0], "history") != 0) {
            addNode(cmd);
        }

        if(isInternalCommand(cmd)) {
            executeInternalCommand(cmd);
        } else {
            pid = fork();

            if(pid == 0) {
                executeCommand(cmd);
            } else if(pid > 0){
                child = waitpid(pid, &status, 0);
            } else {
                printf("There was an error\n");
            }
        }

        free(cmdLine);
        int walk = 0;
        while(cmd[walk] != NULL) {
            free(cmd[walk]);
            walk++;
        }
        free(cmd);
    }

    return 0;

}

This is what calls the traverselist function

/**
 * Executes the given internal command; Nothing is returned since this is all executed on the address space
 * @param cmd The command to be executed
 */
void executeInternalCommand(char** cmd) {
    if(strcmp(cmd[0], "exit") == 0) {
        exit(0);
    } else if(strcmp(cmd[0], "cd") == 0) {
        if(cmd[1] != NULL) {
            chdir(cmd[1]);
        }
    } else if(strcmp(cmd[0], "echo") == 0) {
        int temp_index = 1;
        while(cmd[temp_index] != NULL) {
            if(cmd[temp_index + 1] != NULL) {
                snprintf(message, WRITE_BUFFER, "%s ", cmd[temp_index]);
                write(STDOUT_FILENO, message, strlen(message));
            } else if(cmd[temp_index] != NULL && cmd[temp_index + 1] == NULL) {
                snprintf(message, WRITE_BUFFER, "%s\n", cmd[temp_index]);
                write(STDOUT_FILENO, message, strlen(message));
            } else {
                snprintf(message, WRITE_BUFFER, "\n");
                write(STDOUT_FILENO, message, strlen(message));
            }
            temp_index++;
        }
    } else if(strcmp(cmd[0], "help") == 0) {
        if(cmd[1] == NULL) {
            snprintf(message, WRITE_BUFFER, "GNU bash, version 5.0.17(1)-release (x86_64-pc-linux-gnu)\n");
            write(STDOUT_FILENO, message, strlen(message));
            snprintf(message, WRITE_BUFFER, "These shell commands are defined internally. 'Type help' to see this list.\n");
            write(STDOUT_FILENO, message, strlen(message));
            snprintf(message, WRITE_BUFFER, "Type 'help name' to find out more about the function 'name'.\n\n");
            write(STDOUT_FILENO, message, strlen(message));
            snprintf(message, WRITE_BUFFER, "%-30s %-30s\n", "cd [File Path]", "help name");
            write(STDOUT_FILENO, message, strlen(message));
            snprintf(message, WRITE_BUFFER, "%-30s %-30s\n", "echo [Text]", "exit");
            write(STDOUT_FILENO, message, strlen(message));
        } else {
            if(strcmp(cmd[1], "exit") == 0) {
                snprintf(message, WRITE_BUFFER, "exit\nExits the program with status code 0\n");
                write(STDOUT_FILENO, message, strlen(message));
            } else if(strcmp(cmd[1], "cd") == 0) {
                snprintf(message, WRITE_BUFFER, "cd [File Path]\nChange the current directory. Accepts argument 'File Path' to change the present working directory to the 'File Path'\n");
                write(STDOUT_FILENO, message, strlen(message));
            } else if(strcmp(cmd[1], "echo") == 0) {
                snprintf(message, WRITE_BUFFER, "echo [Text]\nPrints [Text] to the screen\n");
                write(STDOUT_FILENO, message, strlen(message));
            } else {
                snprintf(message, WRITE_BUFFER, "The help page for this command is currently not supported\n");
                write(STDOUT_FILENO, message, strlen(message));
            }
        }
    } else if(*cmd[0] == '!') {
        printf("Banger!!!!\n");
    } else if(strcmp(cmd[0], "history") == 0) {
        printf("History command\n");
        traverseList();
    }
}

EDIT: I had a very rookie mistake. In my addNode function I allocated space in there for my returnNode AND returning that. So when I return a local variable from a function and try to use it outside of the method scope I will obviously have messed up memory references. Just a big rookie mistake there messing up with scopes.

Upvotes: 0

Views: 55

Answers (1)

selbie
selbie

Reputation: 104524

This isn't the root cause of your crash, but this is a bug. You are leaking an object here in your addnode function:

void addNode(char** command) {
    struct Node* tempNode = createNode(command);

    if(head == NULL) {
        head = malloc(sizeof(struct Node));
        head = tempNode;
    }

Should be:

    if(head == NULL) {
        head = tempNode;
    }

Upvotes: 2

Related Questions