fgets() Overwriting part of a struct's data

Question

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.

There is a not-so-slight problem with your code. Suppose you want to add 10,000 students. That needs a lot of memory! Where does all that memory come from? — n. m. could be an AI
– n. m. could be an AI, Commented Oct 1, 2024 at 5:39
The assignment *start = &temp is wrong... You make *start point to the local variable temp. Once the function returns, temp no longer exists and the pointer will become invalid. — Some programmer dude
– Some programmer dude, Commented Oct 1, 2024 at 5:44
Also be careful with strncpy, there's a case where it might not actually null-terminate the destination string. — Some programmer dude
– Some programmer dude, Commented Oct 1, 2024 at 5:45
Do you know by struct student { . . . } Student; you actually made an object of type student named Student and never used it? — Iman Abdollahzadeh
– Iman Abdollahzadeh, Commented Oct 1, 2024 at 6:10
Maybe not directly related to your problem, but I suggest you closely read the documentation of strncpy, it most likely does not what you think it does. Or in other words: strncpy is not the safer version of strcpy — Jabberwocky
– Jabberwocky, Commented Oct 1, 2024 at 7:09

chux · Accepted Answer · 2024-10-01 11:52:13Z

2

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)

edited Oct 1, 2024 at 11:52

chux

158k17 gold badges160 silver badges310 bronze badges

answered Oct 1, 2024 at 7:03

jmsapt

1205 bronze badges

Sign up to request clarification or add additional context in comments.

Collectives™ on Stack Overflow

fgets() Overwriting part of a struct's data

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related