1

I am trying to create a linked list which stores name and age of a student. I am having trouble with insertion.

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

typedef struct node{

  char Name[50];
  int studentAge;
  struct node* next;

}MyNode;

this is how i defined my Struct which constains the requried data and a pointer 'next' which points to the next node.

Below is my insertion function so in the first if condition i am saying if there isnt a head ie head = NULL then create memory space for the head using malloc.. after this i copy all the data into the head node and making sure that the next of head points to null.

In the second condition i am saying if there is a head ie Head ! = NULL then traverse the list to the end using the current pointer and then copy all the data in.

void InsertStudent(char givenName[50], int age, MyNode* head){

    if(head == NULL){
        head = (MyNode*) malloc(sizeof(MyNode));
        strcpy(head->Name,givenName);
        head->studentAge = age;
        head->next = NULL;
    }


    if(head != NULL){
        MyNode* current = head;
            while(current->next != NULL){
                current = current->next;
            }
        current->next = (MyNode*) malloc(sizeof(MyNode));
        strcpy(current->next->Name,givenName);
        current->next->studentAge = age;
        current->next->next = NULL;
    }

}

Now i am not sure if there is a problem in my printing or inserting because it doesn't print my nodes when i try the code out

void PrintList(MyNode* head){
    MyNode* current = head;

    while(current != NULL){
        printf("Name is %s Age is %d\n",current->Name,current->studentAge);
        current = current->next;
    }

}

this is my main function.. is there a problem with the MyNode* head = NULL; line of code is that allowed?

  int main()
   {


    MyNode* head = NULL;

    int r = 0;
while(r!=1)
    {
    printf("Data Structures - Linked List\n");
    printf("Choose one Option:\n\n");
    printf("1.Insert Student\n");
    printf("2.Remove Student\n");
    printf("3.Print all student\n");
    printf("4.Exit\n");

        int option=0;
        char givenName[50];
        int givenAge;
        scanf("%d",&option);

        switch(option){

        case 1:
        printf("Enter name of student:     ");
        scanf("%s",givenName);
        printf("\nEnter Age of student:    ");
        scanf("%d",&givenAge);
        InsertStudent(givenName,givenAge,head);
            break;

        case 2:
        printf("Enter name of student:     ");
        scanf("%s",givenName);
        printf("\nEnter Age of student:    ");
        scanf("%d",&givenAge);
        RemoveStudent(givenName,givenAge);
            break;

        case 3:
        PrintList(head);
            break;
        case 4:
        r=1;
            break;
        default:
        r=1;
        printf("\nNot an option\n");
            break;

       }

    }
}
EEECS
  • 13
  • 2
  • 1
    `head = (MyNode*) malloc(sizeof(MyNode))` in `InsertStudent` means *nothing* to the caller of that function. The `head` pointer is passed by value. Assigning to it as such just modifies a local variable; the caller's pointer remains unchanged. Either utilize the otherwise-unused return value of the function to communicate a possibly-updated head pointer, or pass the head pointer by *address* (so a pointer-to-pointer) and change the code in `InsertStudent` accordingly. There are at least a thousand duplicates of this problem on SO. I'll try and hunt one down. – WhozCraig Nov 23 '18 at 20:50
  • Found one [**here**](https://stackoverflow.com/questions/37982512/push-operation-on-stack-using-linked-list-fails). The first and second answers actually demonstrate both methods I described above. – WhozCraig Nov 23 '18 at 20:52
  • from what i have seen, are you trying to suggesting to either change void Insert(..) to MyNode* insert(...) and return the new node... or the second method is to use void ( double pointer)... is that what your saying? i would like to go with double pointers as that seems harder but i don't really know how to do it.. – EEECS Nov 23 '18 at 20:55
  • The comment I posted, in conjunction with the answer and prior question I linked, cannot possibly describe better what needs to be done. The second answer in the linked question shows *precisely* how to do this with pointer-to-pointer syntax. – WhozCraig Nov 23 '18 at 20:56
  • thanks. ill fix it now. i just saw ur second post so i didnt know you linked an example – EEECS Nov 23 '18 at 20:59
  • Adding to what WhozCraig said, consider defining a type `List` to be a pointer `MyNode*` and using that instead. This makes it easier to understand what is going on, in my opinion. If you call `InsertStudent(name, age, list)` none of the three arguments will come out with a different value, including the pointer `list`. If `list` was null before the call it will be null after the call. – Jorge Adriano Branco Aires Nov 23 '18 at 21:05
  • JorgeAdriano i don't really understand what your saying... i'm new to pointers. @WhozCraig i changed it to Insert(name,age, MyNode** head) and input is Insert(name,age,&head) but now i am getting errors with strcpy(head->name,givenname) and head->studentage = age i'm not sure why – EEECS Nov 23 '18 at 21:15

2 Answers2

0

You're not setting the initial value of the head pointer to the first node, and since that is never done, the list remains empty and you leak memory like a sieve leaks rain water.

As you have communicated you want to use pointer-to-pointer syntax, the result should look like this . (sans error checking, which you shoudl probably consider adding):

void InsertStudent(char givenName[50], int age, MyNode** head)
{
    while (*head)
        head = &(*head)->next;

    *head = malloc(sizeof **head);
    strcpy((*head)->Name, givenName);
    (*head)->studentAge = age;
    (*head)->next = NULL;
}

Invoked from your main program using the address of the head pointer (do NOT confused that with the address held in the head pointer which you're initially setting to NULL correctly; think of the latter a value held by a pointer, the former as a residence where the head pointer itself is in memory).

InsertStudent(givenName,givenAge, &head); // NOTE THIS

I leave the task of removal and list cleanup.

WhozCraig
  • 65,258
  • 11
  • 75
  • 141
  • sorry to bother you.. i dont understand what these 3 lines do: while (*head) head = &(*head)->next; *head = malloc(sizeof **head); – EEECS Nov 23 '18 at 21:42
  • Remember, you're passing the *address of a pointer*. It's therefore coming in as a pointer-to-pointer. if `head` in this function is a pointer to a pointer, then `*head` is the value of the pointer being pointed to. That's the original pointer in `main`. I strongly suggest you hit up some google-fu with the topic of "C pointer to pointer". or consult your text if you have one. And, as always, single stepping though this code with a debugger, watching variables as it progresses, is *extremely* educational. – WhozCraig Nov 23 '18 at 21:45
  • in insert(**head) am i right in assuming head is actually address of head so is it ok to change head into addresshead in that function? then *addresshead will actually be head and **addresshead would be the actual first node, it doesnt really affect the code but its confusing like this.. i am just making sure my logic is correct – EEECS Nov 23 '18 at 22:38
  • It sounds like it's more or less accurate. – WhozCraig Nov 24 '18 at 05:52
  • i kind of followed what you said and thought about it and now its working i think i slightly understand double pointers now.. Thanks – EEECS Nov 24 '18 at 14:42
0

You are passing head by value; which means that the line in InsertStudent:

head = (MyNode*) malloc(sizeof(MyNode))

which does not update the variable ‘head’ in main. What you want is to pass &head to InsertStudent, but then InsertStudent has to deal with a MyNode **. The other option is have InsertStudent return head, so that its invocation is:

 head = InsertStudent(name, age, head);

It doesn’t matter much either way, some people prefer the latter because it looks more functional.

Inside of InsertStudent, you add the first element twice. This is almost certainly unwanted. By the time you get to the line:

if(head != NULL){

head is never NULL; if it were, you would have assigned it in the if statement above. You probably want this statement to be:

else {
mevets
  • 10,070
  • 1
  • 21
  • 33