What's wrong with my "Create new nodes" function & my "add node as last node" function?

Question

I have 2 functions, CreateNewNode, a function that creates a new node, and Add_as_Last_Node, a function that adds the node created in CreateNewNode as the last node in the linked list. While running the program I had made using these two functions, the output was not I had expected.

For example, if I was the user, and I wanted to make a linked list of 5 nodes, and my inputs are "apple", "bag", "car", "dress", and "elephant", when I traverse and print the nodes the output is:

elephant elephant elephant elephant elephant

When it should be:

apple bag car dress elephant

After studying my codes for hours, I have deciphered that the problem of my program lies in either CreateNewNode, or Add_as_Last_Node. Or maybe both. I've tried tracing it, but with no luck of finding what's wrong with it. Can someone please help me in finding what wrong? Thank you! :)

typedef char Str30[31];

struct nodeTag {
   char *data;
   struct nodeTag *pNext;
};

void Traverse(struct nodeTag *pCurrent)
{
   while(pCurrent){  // while there's a node
      printf("%s\n", pCurrent->data);
      pCurrent = pCurrent->pNext; // move to the next node
   }
}

struct nodeTag *CreateNewNode(Str30 data)
{
   struct nodeTag *pTemp;

   //create and initialize a new node
   pTemp = malloc(sizeof(struct nodeTag));
   pTemp->data = data;
   pTemp->pNext = NULL;

   return pTemp;
}

struct nodeTag *Add_as_Last_Node(struct nodeTag *pFirst, struct nodeTag *pTemp)
{
    struct nodeTag *pCurrent;

    if(pFirst == NULL){ // list is empty
       pFirst = pTemp;
    }
    else {
       pCurrent = pFirst;

       while(pCurrent->pNext != NULL)
          pCurrent = pCurrent->pNext;

       pCurrent->pNext = pTemp;
    }

    return pFirst;
}

int main()
{
   Str30 data;
   struct nodeTag *pFirst = NULL;
   struct nodeTag *pTemp;

   while(scanf("%s", data) != -1){
      pTemp = CreateNewNode(data);
      pFirst = Add_as_Last_Node(pFirst, pTemp);
   }

   Traverse(pFirst);

   return 0;
}

This is the actual code I am using:

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

typedef char Str30[31];

struct nodeTag {
   char *data;
   struct nodeTag *pNext;
};

void Traverse(struct nodeTag *pCurrent)
{
   // traverse the linked list
   while(pCurrent){  // while there's a node
     printf("%s\n", pCurrent->data);
     pCurrent = pCurrent->pNext; // move to the next node
   }
}

struct nodeTag *CreateNewNode(Str30 data)
{
   struct nodeTag *pTemp;

   //create and initialize a new node
   pTemp = malloc(sizeof(struct nodeTag *));
   //strcpy(pTemp->data, data);
   pTemp->data = data;
   pTemp->pNext = NULL;

   return pTemp;
}

struct nodeTag *Add_as_Last_Node(struct nodeTag *pFirst,
                             struct nodeTag *pTemp)
{
  struct nodeTag *pCurrent;

  if(pFirst == NULL){ // list is empty
     pFirst = pTemp;
  }
  else {
     pCurrent = pFirst;

     while(pCurrent->pNext != NULL)
        pCurrent = pCurrent->pNext;

     pCurrent->pNext = pTemp;
  }

  return pFirst;
}

struct nodeTag *Insert_Sort(struct nodeTag *pFirst,
                        struct nodeTag *pTemp)
{
    struct nodeTag *pTrail;
    struct nodeTag *pCurrent;

    if(pFirst == NULL)
        pFirst = pTemp;
    else{
        pTrail = NULL;
        pCurrent = pFirst;

        while(pCurrent->pNext != NULL){
            pTrail = pCurrent;
            pCurrent = pCurrent->pNext;
        }

        pTemp->pNext = pCurrent;

        if(pTrail != NULL)
            pTrail->pNext = pTemp;
        else
            pFirst = pTemp;
    }

    return pFirst;
}

int main()
{
    int ctr = 0;
    Str30 data;
    // Str30 universe;
    struct nodeTag *pFirst = NULL;
    struct nodeTag *pTemp;

    while(ctr != 3 && scanf("%s", data) != -1){
        printf("\ndata = %s\n", data);
        pTemp = CreateNewNode(data);
        printf("\nAfter CreateNewNode: \n\n");
        Traverse(pFirst);
        pFirst = Add_as_Last_Node(pFirst, pTemp);
        printf("\nAfter Add_as_Last_Node: \n\n");
        Traverse(pFirst);
        ctr++;

    }

    Traverse(pFirst);

    printf("\n%d\n", ctr);

    return 0;
}

Right now that is what I am using to test what is wrong with my codes.

Answer 1

UPDATE:

My codes are finally working. You guys were right about strcpy(temp->data, data). The real reason why it didn't work was because of other parts in the code.

Anyway, thanks for the help! I'll check everyone else in thanks. Danke! :D

Answer 2

1. Change your pTemp->data = data; to strcpy(pTemp->data, data).
2. while(scanf("%s", data) != -1) ..looks suspicious. check the man page of scanf()

For more information, please post a MCVE

---

EDIT:

Please check the below code.

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


typedef char Str30[31];

struct nodeTag {
        Str30 data;
        struct nodeTag *pNext;
};
#if 1
void Traverse(struct nodeTag *pCurrent)
{
   while(pCurrent){  // while there's a node
      printf("%s\n", pCurrent->data);
      pCurrent = pCurrent->pNext; // move to the next node
   }
}

struct nodeTag *CreateNewNode(Str30 data)
{
        struct nodeTag *pTemp;

        //create and initialize a new node
        pTemp = malloc(sizeof(struct nodeTag));
        strcpy(pTemp->data, data);
        pTemp->pNext = NULL;

        return pTemp;
}

struct nodeTag *Add_as_Last_Node(struct nodeTag *pFirst, struct nodeTag *pTemp)
{
        struct nodeTag *pCurrent;

        if(pFirst == NULL){ // list is empty
                pFirst = pTemp;
        }
        else {
                pCurrent = pFirst;

                while(pCurrent->pNext != NULL)
                        pCurrent = pCurrent->pNext;

                pCurrent->pNext = pTemp;

        }
                return pFirst;
}

int main()
{
        Str30 data;
        struct nodeTag *pFirst = NULL;
        struct nodeTag *pTemp = NULL;
        int count = 5;

        while(count--)
        {
                memset(data, 0, sizeof(data));
                scanf("%s", data);
                pTemp = CreateNewNode(data);
                if (!pTemp)
                {
                    printf("pTemp is NULL\n");
                }
                pFirst = Add_as_Last_Node(pFirst, pTemp);
        }
        Traverse(pFirst);

        return 0;
}

Answer 3

The typedef you have used results in an array type. Try changing it (typedef char Str30[31];) to use a struct

typedef struct Str30Type { char value[30]; } Str30Type;

ArrayTypes when used as function arguments will be passed by reference and not by value. So I think you are copying reference of the same object 5 times over and using the last updated value.

typedef char Str30[31];

struct nodeTag {
   Str30 data;
   struct nodeTag *pNext;
};

void Traverse(struct nodeTag *pCurrent)
{
   while(pCurrent){  // while there's a node
      printf("%s\n", pCurrent->data);
      pCurrent = pCurrent->pNext; // move to the next node
   }
}

struct nodeTag *CreateNewNode(Str30 data)
{
   struct nodeTag *pTemp;

   //create and initialize a new node
   pTemp = malloc(sizeof(struct nodeTag));
   memcpy(pTemp->data,data,strlen(data));
   pTemp->pNext = NULL;

   return pTemp;
}

struct nodeTag *Add_as_Last_Node(struct nodeTag *pFirst, struct nodeTag *pTemp)
{
    struct nodeTag *pCurrent;

    if(pFirst == NULL){ // list is empty
       pFirst = pTemp;
    }
    else {
       pCurrent = pFirst;

    while(pCurrent->pNext != NULL)
       pCurrent = pCurrent->pNext;

    pCurrent->pNext = pTemp;

    return pFirst;
    }
}

int main()
{
   Str30 data;
   struct nodeTag *pFirst = NULL;
   struct nodeTag *pTemp;

   while(scanf("%s", data) != -1){
      pTemp = CreateNewNode(data, strlen(data));
      pFirst = Add_as_Last_Node(pFirst, pTemp);
   }

   Traverse(pFirst);

   return 0;
}

Answer 4

Multiple things are wrong here:

pTemp->data = data;

This is wrong you need to use strcpy().

Check the code below:

#include<stdio.h>
#include<stdlib.h>
#include<string.h>
   typedef   char Str30[31];

 struct nodeTag {
      Str30 data;
      struct nodeTag *pNext;
   }; 
    void Traverse(struct nodeTag *p)
    {
       struct nodeTag *t = p;
       while(t != NULL)
       {   
          printf("%s\n",t->data);
          t = t->pNext;
       }   
       return;
    }
    struct nodeTag *CreateNewNode(Str30 data)
    {
       struct nodeTag *pTemp;


       pTemp = malloc(sizeof(struct nodeTag));
       strcpy(pTemp->data, data);
       pTemp->pNext = NULL;

       return pTemp;
    }

    void Add_as_Last_Node(struct nodeTag *pFirst, struct nodeTag *pTemp)
    {
       struct nodeTag *pCurrent;

          pCurrent = pFirst;

          while(pCurrent->pNext != NULL)
             pCurrent = pCurrent->pNext;

          pCurrent->pNext = pTemp;
    }
       int main()
       {   
          Str30 data;
          int i=0;
          struct nodeTag *pFirst = NULL;
          struct nodeTag *pTemp;
          for(i=0;i<3;i++)
          {   
             scanf("%s",data);
             pTemp = CreateNewNode(data);
             if(pFirst == NULL)
             {   
                pFirst = pTemp;
             }   
             else
             {   
                Add_as_Last_Node(pFirst, pTemp);
             }   
          }   
          Traverse(pFirst);

          return 0;
       }