CODEWITHSUNDEEP
clinked-list
Zed
Zed

Reputation: 51

Linked list, an element gets lost

I'm trying to insert a node to the end of the linked list and everything works like wonder, however, when it comes to last element it prints null. There problem must be in the else block, I was thinking that when I look for the last item it may points to NULL instead to the element that have next as NULL 

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


#define HOW_MANY 7
char *names[HOW_MANY]= {"Simon", "Suzie", "Alfred", "Chip", "John", "Tim",
              "Harriet"};
int ages[HOW_MANY]= {22, 24, 106, 6, 18, 32, 24};

struct person {
  int age;
  char *name;
    struct person *next;
};


struct person* insert_end(struct person *ptr, char *name, int age)
{
  //The new location for the new person
  struct person* newPer = malloc(sizeof(struct person));
    if (newPer == NULL) {
        printf("something went wrong allocating the memory\n");
        exit(1);
    }

  newPer -> name = name;
  newPer -> age = age;
  //Make its next points previous element
  if (ptr->next == NULL) {
    newPer -> next = ptr;


    return newPer;
  }
  else {
    struct person *tmp = ptr;
    while(tmp -> next != NULL){
      tmp = tmp -> next;

    }
    printf("%s", tmp -> name);
    tmp -> next = newPer;


    //if(strcmp("Harriet",name)==0)
    return ptr;
  }
  //return the new address so that it becomes the new HEAD of the linked list
    return newPer;
}


int main(int argc, char **argv)
{
    //This is the head of the list
    struct person *HEAD = NULL;
    HEAD = malloc(sizeof(struct person));
    if (HEAD == NULL) {
        printf("something went wrong allocating the memory");
        exit(1);
    }


    int i;
  //insert a new person and make HEAD points to it, so that HEADS
  //will be pointing to the last element added to the linked list.
  for (i = 0; i < HOW_MANY; i++){
        HEAD = insert_end (HEAD, names[i], ages[i]);
    }

  struct person* tmp = HEAD;
  //We can use the member name(pointer) as the condition, if we did then extra
  //unwanted elements added to the linked list by accident won't be printed
  while (tmp != NULL){
    if (tmp -> next != NULL)
      printf("The name is %s, the age is %d years\n", tmp->name, tmp->age);

    //store the pointer in a tmp so than we can access the next pointer then
    //free tmp
    struct person* prevtmp = tmp;
    tmp = tmp -> next;
    free(prevtmp);
  }
}

The output is

The name is Simon, the age is 22 years
The name is (null), the age is 0 years
The name is Suzie, the age is 24 years
The name is Alfred, the age is 106 years
The name is Chip, the age is 6 years
The name is John, the age is 18 years
The name is Tim, the age is 32 years

Upvotes: 1

Views: 129

Answers (4)

jwdonahue
jwdonahue

Reputation: 6659

First off, an uninitialized list head should always be NULL. The insert functions should detect this and simply return the properly initialized first node. After that, you can base your logic on whether p->next is NULL. You pretty much have to rewrite a good bit of your code to get it right. For 'insert_end' should always return the head pointer it was handed, 'insert_front' should always return the new node because it is always the new head.

Upvotes: 0

ividito
ividito

Reputation: 336

You initialize *HEAD as NULL, but you never populate that address with a person, so that NULL gets pushed to the end and stays there. Try adding this at the top of your function:

if (*ptr == NULL){
    newPer = ptr
}
else {
    struct person* newPer = malloc(sizeof(struct person));
    if (newPer == NULL) {
        printf("something went wrong allocating the memory\n");
        exit(1);
    }
}

You might have to change a couple of other conditions, but the idea is that you need to check if newPer is the first entry in the list, and change how you handle that case.

Upvotes: 0

jxh
jxh

Reputation: 70402

Make sure newPer->next is set to NULL.

  newPer -> name = name;
  newPer -> age = age;
  newPer -> next = NULL;

Same for your sentinel HEAD.

    HEAD = malloc(sizeof(struct person));
    if (HEAD == NULL) {
         printf("something went wrong allocating the memory");
        exit(1);
    }
    HEAD->name = "HEAD";
    HEAD->age = -1;
    HEAD->next = NULL;

Upvotes: 1

mnistic
mnistic

Reputation: 11020

Simply change if (tmp -> next != NULL) to if (tmp != NULL). You forgot that the last node in the list will not have a "next" node.

Upvotes: 1

Related Questions