Why aren't the elements of my Doubly Linked List displayed properly?

Question

The following program receives input strings of the form ins "name_to_insert" birthdate and should insert this information in a doubly linked list. The contents of the list are displayed after each insertion, along with the number of elements. The number of elements is being displayed correctly, but instead of the names and birth dates, 2686707 is being displayed n times (n=number of elements in the list).

I suspect something is wrong with my print function, printList(), but I couldn't figure out what.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "DLList.h"

typedef struct dataStructure
{
    int birthday;
    char *name;
} dataStructure;

int main()
{
    ListT *l=createList();
    char op[4], nameTemp[30], *name, s[50];
    int date;
    while (scanf("%[^\n]%*c", s)==1)
    {
        sscanf(s, "%s", op);
        if (strcmp(op, "ins")==0)
        {
            sscanf(s, "%*s %[^0-9]%d", nameTemp, &date);
            name=nameTemp+1; // Remove opening quotation mark
            name[strlen(name)-2]='\0'; // Remove closing quotation mark
                NodeT *p=createNode();
                p->data=(dataStructure*)malloc(sizeof(dataStructure));
                ((dataStructure*)p->data)->birthday=date;
                ((dataStructure*)p->data)->name=name;
                insertLastNode(l, p);
                printf("List length: %d\n", l->length);
                printList(l);
        }
    }
    return 0;
}

void printList(ListT *l)
{
    NodeT *p=l->first;
    while (p)
    {
        printf("%d %s\n", (((dataStructure*)p->data)->birthday, (dataStructure*)p->data)->name);
        p=p->next;
    }
    printf("--\n");
}

Contents of DLList.h:

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

typedef struct nodetype
{
    struct nodetype *prev, *next;
    void *data;
} NodeT;

typedef struct
{
    int length;
    NodeT *first, *last;
} ListT;

NodeT *createNode();
ListT *createList();
void insertLastNode(ListT *l, NodeT *p);

Contents of DLList.c:

#include "DLList.h"

NodeT *createNode()
{
    NodeT *p=(NodeT*)malloc(sizeof(NodeT));
    p->next=p->prev=NULL;
    return p;
}

ListT *createList()
{
    ListT *l=(ListT*)malloc(sizeof(ListT));
    l->first=l->last=NULL;
    l->length=0;
    return l;
}

void insertLastNode(ListT *l, NodeT *p)
{
    if (l->first==NULL)
    {
        l->first=l->last=p;
        p->prev=p->next=NULL;
        l->length++;
    }
    else
    {
        p->prev=l->last;
        p->next=NULL;
        l->last->next=p;
        l->last=p;
        l->length++;
    }
}

Answer 1

Two errors in the code.

First: Instead of

((dataStructure*)p->data)->name=name;

do

((dataStructure*)p->data)->name = (char*)malloc(strlen(name)+1);
strcpy(((dataStructure*)p->data)->name, name);

to avoid memory leakage.

Second In printList function there was a mistake with parentheses. Do as below:

printf("%d %s\n", ((dataStructure*)p->data)->birthday, ((dataStructure*)p->data)->name);

Everything else works. Here is output I got when tested the program.

Answer 2

Run this in a debugger to see exactly what your function is doing. For example if you're running from a Unix command line, compile with the -g flag, then run in gdb. Set a breakpoint at the beginning of the function that is troubling you. Google something like "gdb cheat sheet" for details, it's easy.

Answer 3

In your program, you are assigning the pointer to name as below:

((dataStructure*)p->data)->name=name;

This name is derived from the sscanf as below:

sscanf(s, "%*s %[^0-9]%d", nameTemp, &date);
name=nameTemp+1; // Remove opening quotation mark
name[strlen(name)-2]='\0'; // Remove closing quotation mark

This means that for every run of the loop, you are reading into the same nameTemp or name array and storing the same into the linked list. For every run of the loop, you may have to allocate a separate space for storing the name and assign the same to your node.

EDIT 1:

When you create a new node, there is also one type-casting issue. p->data is of void * type, in the code, I believe, the newly allocated memory is type-casted as dataStructure * as below

p->data=(dataStructure*)malloc(sizeof(dataStructure));

You could also change your data structure definition as below

typedef struct dataStructure
{
    int birthday;
    char name[64]; // Change from pointer to an array
} dataStructure;

And modify the logic in the loop to copy the name as below:

p->data=(dataStructure*)malloc(sizeof(dataStructure));
((dataStructure*)p->data)->birthday=date;
strcpy(((dataStructure*)p->data)->name, name); // Modified from pointer assignment to strcpy