Fahad Saleem
Fahad Saleem

Reputation: 423

Strings are being overwritten in a linked list

I have already checked out previous answers on the same topic given on this website, but still my error is not going. The program is about a student management system, in which users dynamically add new student information that includes name, degree and age. When I display all the information of the nodes, the age is being displayed correctly but the name and degree are overwritten by the last node.

    #pragma warning(disable: 4996)
    #include <stdio.h>
    #include <conio.h>
    #include <malloc.h>
    #include <Windows.h>




struct students{
    char *name;
    int age;
    char *degree;
    struct students* next;

};

int TotalStudents = 0;

struct students* ptrToHead;

void insertAtBeginning(char name[], int age, char degree[]){

    struct students * temp = (students *)malloc(sizeof(struct students));


    temp->name= name;
    temp->age = age;
    temp->degree=degree;
    temp->next = NULL;
    if (ptrToHead != NULL)
    {
        temp->next = ptrToHead;
    }
    ptrToHead = temp;

    //printf("%s\n%d\n%s", temp->name, temp->age, temp->degree);
}

void print(){

    struct students* temp = ptrToHead;
    printf("List of Students: ");
    while (temp != NULL){
        printf("\nStudent's Name: %s", temp->name);
        printf("\nStudent's Age: %d", temp->age);
        printf("\nStudent's Degree: %s", temp->degree);
        printf("\nEND - OF - STUDENT");
        temp = temp->next;

    }
    printf("\n");
}

void MainMenu();
void addStudent();


int main(){

    MainMenu();

    //students * temp= (students *)malloc(sizeof(students));

    //temp->age = 22;
    //temp->degree = "Software Engineering";
    //temp->name = "Fahad Bin Saleem";
    //temp->next = NULL;

    //ptrToHead = temp;

    //

    //printf("Age: %d\n", ptrToHead->age);
    //printf("Name: %s\n", ptrToHead->name);
    //printf("Degree: %s\n", ptrToHead->degree);



    //temp = (students *)malloc(sizeof(students));
    //temp->age = 19;
    //temp->degree = "Electrical Engineering";
    //temp->name = "Rafay Hayat Ali";
    //temp->next = NULL;



    //students * temp1 = ptrToHead;

    //while (temp1->next != NULL){
    //  temp1 = temp1->next;


    //}
    //temp1->next = temp;
    //









    _getch();
    return 0;
}

void MainMenu(){
    int choice;
    printf("Welcome to Student Information Center!\n\n");
    char* mainmenu[] = { "Display All Students", "Add A Student" };

    for (int i = 0; i < 2; i++){
        printf("%d:  %s\n", i + 1, mainmenu[i]);
    }
    printf("\n\nEnter Your Choice: ");
    scanf_s(" %d", &choice);

    if (choice == 2){
        addStudent();
    }
    if (choice == 1){
        print();
    }


}

void addStudent(){
    int NumberOfStudents;
    int choiceOfAdding;
    char tempName[40];
    char tempDegree[40];
    int tempAge;
    system("cls");



    ptrToHead = NULL;

    for (int i = 0; i < 15; i++){
        printf("  ");
    }
    printf("**ADD A STUDENT**");

    printf("\n\nHow many students do you want to add? Enter Choice: ");
    scanf_s(" %d", &NumberOfStudents);

    printf("\n\n");

    for (int i = 0; i < NumberOfStudents; i++){
        printf("\n\n");


        printf("Enter Student's Name:  ");
        fflush(stdin);
        gets_s(tempName, 40);
        printf("Enter Student's Age:  ");
        scanf_s(" %d", &tempAge);
        printf("Enter Student's Degree:  ");
        fflush(stdin);
        gets_s(tempDegree, 40);
        //insert(tempName, tempAge, tempAgeDegree);


        //printf("Where Do You Want To Add This Student?\n\n1: At The Beginning\n\n2: At A Position N\n\n3: At The End");
        //scanf_s(" %d", &choiceOfAdding);
        fflush(stdin);
        TotalStudents++;

        insertAtBeginning(tempName, tempAge, tempDegree);
        /*if (choiceOfAdding == 1){

        }*/

        printf("\n\n");

    }
    MainMenu();


}

Upvotes: 2

Views: 416

Answers (1)

ForeverStudent
ForeverStudent

Reputation: 2537

Lets highlight some issues one by one:

In insertAtBeginning you have

    struct students * temp = (students *)malloc(sizeof(struct students));

Don't cast return of malloc in C, read up on it for more details. This is not a fatal error here but bad form regardless.

further down you have temp->name= name;

you are ASSIGNING a char[] as the name instead of allocating necessary memory and copying the contents you don't know the lifetime of the name passed in, the result could be catastrophic. You are assigning a memory location whose contents may change without you knowing, and the name you are storing can change to reflect that. (or worse, the memory location would no longer hold valid information) In fact this is the reason the name gets overwritten every time you "add a new student".

in order to fix this you will need to have:

temp->name= malloc(strlen(name)+1); 
//allocate memory and keep 1 extra for \0
strcpy(temp->name, name);
//copy the value of the parameter into the memory location we just allocated

for temp->degree=degree; you will have the same issue.

more problems: As Kaylum mentioned, you are calling MainMenu and AddStudent within the bodies of each other. While this is acceptable practice in some cases ( like mutual recursion that you know will eventually terminate due to a base case), It is not the behaviour that you want here.

What is happening is you are creating seperate stack frames on top of each other every time you call one of these functions from the other. That means when you have MainMenu->addStudent->MainMenu->addStudent

The original MainMenu stack has not yet resolved and is waiting for all subsequent function calls to return before it itself returns.

If your program goes on for long enough you will surely overflow the stack.

Last thing: try to avoid using global variables when you don't need to.

Upvotes: 2

Related Questions