Caleb Barone
Caleb Barone

Reputation: 11

fgets() Overwriting part of a struct's data

So I am having a bit of trouble with this coding project, It is supposed to be a linked list with a bunch of fake students. I can get the first student in correctly, but when the fgets() runs again it overwrites all of the data in that first struct aside from the first 4 characters. I am rather new to C.

I checked all the pointers thinking that one of them might be the issue, but nothing stood out to me, I was also thinking it could be a memory allocation issue, but I am not really sure how/when exactly I need to do anything with that.

Here is the a link to the code: https://www.onlinegdb.com/EvK1kxplR

and here it is raw

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

struct student
{
    char name[50];
    int age;
    char status[10];
    char password[25];
    struct student* nextStudent;
} Student;

void AddStudent(struct student** start, char name[50], int age, char status[10], char password[25]);
void InsertStudent(struct student** current, struct student newStu);

int main()
{
    struct student* primaris = NULL;
    struct student primFULL;
    char command[100];
    fgets(command, sizeof(command), stdin);
    if(strcmp(command, "db\n") != 0)return 1;
    
    while(1)
    {
        memset(command, 0, sizeof(command));
        printf("db> ");
        if(primaris != NULL)
            primFULL = *primaris;
        fgets(command, sizeof(command), stdin);
        switch(tolower(command[0]))
        {
            case 'q': //quit
                return 0;
            case 'h': //help
                printf("Welcome to COMP-232 Student Database\nValid commands are:\nadd <name>,<age>,<class>,<password>\nwhere:\n\tname is student's name.\n\tage is student's age.\n\tyear is what year the student is in.\n\tpassword is students's password.\ndelete <name> - Deletes name from database.\nlist - Lists all records in database.\nprint <name> - Prints values for name.\nquit - Exits program.\nhelp - Displays this helpful message.\n");
                break;
            case 'a': //add
                char* trash = strtok(command, " ");
                char* name = strtok(NULL, ",");
                int age = atoi(strtok(NULL, ","));
                char* stat = strtok(NULL, ",");
                char* pWord = strtok(NULL, ",");
                AddStudent(&primaris, name, age, stat, pWord);
                primFULL = *primaris;
                break;
            case 'l': //list
                struct student* current = primaris;
                while(current != NULL)
                {
                    primFULL = *current;
                    printf("%s", current->name);
                    current = current->nextStudent;
                }
                break;
        }
    }
}

void AddStudent(struct student** start, char name[50], int age, char status[10], char password[25])
{
    struct student temp;
    strncpy(temp.name, name, sizeof(temp.name));
    temp.age = age;
    strncpy(temp.status, status, sizeof(temp.status));
    strncpy(temp.password, strtok(password, "\n"), sizeof(temp.password));
    temp.nextStudent = NULL;
    if(*start == NULL)
    {
        *start = &temp;
        printf("%p", (*start)->nextStudent);
        return;
    }
    while((*start)->nextStudent != NULL)
    {
        if(strcmp(temp.name, (*start)->nextStudent->name) < 0)
        {
            InsertStudent(start, temp);
            return;
        }
        start  = &(*start)->nextStudent;
    }
    
    return;
}

void InsertStudent(struct student** current, struct student newStu)
{
    newStu.nextStudent = *current;
    (*current)->nextStudent = &newStu;

}

primaris is the start of the linked list, primFULL is just there because I couldn't see what was happening in the debugger I was using.

Upvotes: 1

Views: 83

Answers (1)

jmsapt
jmsapt

Reputation: 120

Echoing what others have said, the issue is that you are relying on a dangling pointer (a pointer to memory that doesn't exist anymore), as you are using a pointer to the stack variable temp.

Just as @Gerhard pointed out, this should be allocated memory using malloc as you want this memory to live longer than the stack frame for the function (which you clearly do).

Bugs like this are not uncommon, especially when you are just starting out. I would suggest using the flag -fsanitize=address or running in valgrind, both of which catch this error. For example, running with -fsantize=address yields the following error, indicating exactly what is happening; we are trying to use old stack memory after returning from that function.

==216298==ERROR: AddressSanitizer: stack-use-after-return on address 0x7f25ca209030 at pc 0x56b42000f747 bp 0x7ffc2b8abac0 sp 0x7ffc2b8abab0
...(etc)

Upvotes: 2

Related Questions