Andy
Andy

Reputation: 13

In linked list, nodes appear to be overwritten by data in last node

I'm in the process of writing a shell for Linux for my Systems Programming class. I've hit a very weird situation in writing a linked list for my command history portion and I'm really not sure what I'm doing wrong.

Right now I have the program set up to accept a char array as input and add the data into a node at the end of a linked list and then set that node as the tail of the list. I then have the program print the list from the tail to head. What I should be seeing is the history of what I've typed in, in reverse order. What I am seeing is that what ever I've typed last appears to be overwriting the data in all of the previous nodes.

Eg: I enter 1, I get a 1 printed back, I then enter 2, I get two 2's printed back at me instead of 2 and then a 1. Does anyone know what is going on? My code is below.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define INPUT_BUFFER 255

typedef struct commandHistoryNode commandHistoryNode;

struct commandHistoryNode{
    commandHistoryNode *previous;
    commandHistoryNode *next;
    char *commandLine;
};

commandHistoryNode* AddCommandToLinkedList(commandHistoryNode *, char *);
void PrintLinkedList();
void ExecuteCommand(char *);

int main(int argc, char **argv){
    char *cmdString;
    cmdString = calloc(INPUT_BUFFER, sizeof(char));
    char *PS1 = "$";
    commandHistoryNode *currentNode = NULL;
    while(1)
    {
        printf(PS1);
        fgets(cmdString, INPUT_BUFFER, stdin);
        //currentNode is the tail of the LinkedList
        currentNode = AddCommandToLinkedList(currentNode, cmdString);

        PrintLinkedList(currentNode);
    }
}

void ExecuteCommand(char *passedCommand){
   return;
}

commandHistoryNode*
AddCommandToLinkedList(commandHistoryNode *passedNode,  char *passedCommand){
    commandHistoryNode *tailNode = malloc(sizeof(commandHistoryNode));
    tailNode->commandLine = passedCommand;
    if(passedNode == NULL)
    {
        //passedNode is the head of the list
        return tailNode;
    }
    else
    {
        while(passedNode->next!=NULL)
        {
            //if passedNode isn't the tail (and it should be), iterate through the list
            passedNode = passedNode->next;
        }
        //set tempNode to the next node for the passedNode
        passedNode->next = tailNode;
        //set the previous node for tempNode to point to the passedNode
        //as it is the new tail.
        tailNode->previous = passedNode;
    }
    //return new tailNode
    return tailNode;
}

void PrintLinkedList(commandHistoryNode *tailNode){
    while(tailNode != NULL)
    {
        printf("command is: %s\n", tailNode->commandLine);
        //iterate backwards from the tail to the head
        tailNode = tailNode->previous;
    }
}

Upvotes: 1

Views: 735

Answers (2)

user2088639
user2088639

Reputation:

You keep re-using the same cmdString for each call to AddCommandToLinkedList. So every node points to the same character buffer. As a result, every time you have a new command, you overwrite the one and only character buffer with that command, and every node points to it. You need to allocate a cmdString for every command, instead of re-using it.

Upvotes: 2

junix
junix

Reputation: 3211

Check again what you return in case passedNode is != NULL.

Upvotes: 0

Related Questions