Linked List - data in previous nodes is being overwritten

Question

I have a program that puts the directories in a linked list and prints it. The linked list is getting messed up with all nodes having the same data. For example, I have a folder "Test' which has 3 subfolders "Good One", "Jump", "Sunday". The output is -> Sunday Sunday Sunday.

On debug, first iteration of insert function, the variable data in the 1st node is set to "Good One". The address of the root node pointer is passed back to the main. Initially, startptr->data = "Good One". After readdir is executed, dir->d_name is "Jump".But startptr->data is getting changed to "Jump". same happens for the 3rd "Sunday" and all the nodes in the list have data = "Sunday".

Not sure why this is happening. Appreciate any help on this. Here is the code:

#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <dirent.h>
#include <sys/stat.h>

/* self-referential structure */
struct listNode {
    char *data; /* each listNode contains a character */
    struct listNode *nextPtr; /* pointer to next node */
}; /* end structure listNode */

typedef struct listNode ListNode; /* synonym for struct listNode */
typedef ListNode *ListNodePtr; /* synonym for ListNode* */

/* prototypes */
void insert( ListNodePtr *sPtr, char *value );
void printList( ListNodePtr currentPtr );

int main( void )
{
    ListNodePtr startPtr = NULL; /* initially there are no nodes */

    DIR *d = NULL;
    struct dirent *dir = NULL;

    d = opendir ("/Users/Satish/Documents/Test");
    if (d != NULL)
    {
        while ((dir = readdir (d)) != NULL)
        {
            /*printf("%s\n", dir->d_name);*/
            if ( (strcmp(dir->d_name, ".")  == 0) || (strcmp(dir->d_name, "..")  == 0) ) 
            {
            }
            else
                insert( &startPtr, dir->d_name ); /* insert item in list */
        }
        closedir (d);
    }
    else
        printf("%s/n", "Couldn't open the directory");

    printList( startPtr );
    printf( "End of run.\n" );
    return 0; /* indicates successful termination */
} /* end main */

/* Insert a new value into the list in sorted order */
void insert( ListNodePtr *sPtr, char *value )
{
    ListNodePtr newPtr = NULL; /* pointer to new node */
    ListNodePtr previousPtr = NULL; /*pointer to previous node in list */
    ListNodePtr currentPtr = NULL; /* pointer to current node in list */

    printf("%s\n", value);
    newPtr = malloc( sizeof( ListNode ) ); /* create node */

    if ( newPtr != NULL )  /* is space available */
    {
        newPtr->data = value; /* place value in node */
        newPtr->nextPtr = NULL; /* node does not link to another node */

        previousPtr = NULL;
        currentPtr = *sPtr;

        if (currentPtr == NULL)
            *sPtr = newPtr;
        else
        {
            while ( currentPtr != NULL ) 
            {
                previousPtr = currentPtr;
                currentPtr = currentPtr->nextPtr;
            }
            previousPtr->nextPtr = newPtr;
            newPtr->nextPtr = NULL;
        }
    }   
    else
    {
        printf( "%c not inserted. No memory available.\n", value );
    } /* end else */

} /* end function insert */


/* Print the list */
void printList( ListNodePtr currentPtr )
{
    /* if list is empty */
    if ( currentPtr == NULL ) {
        printf( "List is empty.\n\n" );
    } /* end if */
    else {
        printf( "The list is:\n" );

        /* while not the end of the list */
        while ( currentPtr != NULL ) {
            printf( "%s\n", currentPtr->data );
            currentPtr = currentPtr->nextPtr;
        } /* end while */

        printf( "NULL\n\n" );
    } /* end else */
} /* end function printList */

Output is:

The list is: Sunday Sunday Sunday

Answer 1

In the insert function you are pointing to the same address for all the elements you have to allocate memory to store the name. see line 57 of your code and add this:

    newPtr = malloc( sizeof( ListNode ) ); /* create node */
   if ( newPtr != NULL )  /* is space available */
   {
    char *val=malloc((strlen(value)+1)*1);
    strcpy(val,value);
    newPtr->data = val; /* place value in node */
    newPtr->nextPtr = NULL;
    .
    .
    .

Answer 2

In insert, you assign the string like this:

newPtr->data = value;

This stores a reference to the string in the dirent struct. This will always be a pointer to the same buffer, but the buffer will have different contents each time you create a link. When you print it, that buffer will of course have the contents of the last entry. (And when the dirent struct goes out of scope the pointer goes stale: It will point to invalid data. That's not the case in your code, though.)

To store the actual value, you must create a copy. Thzat copy need storage, which you must allocate:

newPtr->data = malloc(strlen(value) + 1);
strcpy(newPtr->data, value);

Every malloc needs a corresponding free, so don't forget to free the memory for the string when you destroy your list.