Disclaimer: This is my first question on StackOverflow and I'm a novice programmer, so my apologies ahead of time if you are appalled by my code or if I don't post my question appropriately.
Anyways, I'm working on a gradebook with dynamically allocated structures. I've divided the gradebook into three structues, a student structure (student name, student id), a course structure (course name, course id), and an enroll structure(student id, course id, grade).
Problem: I'm able to input as many grade as I need without errors for the first student. When I try to input a grade for the second student my program core dumps. I've checked all the variables involved to see if they are passed to my function appropriately and they are. The following is my Enroll structure and my add grade function.
typedef struct {
int Student_ID;
int Course_ID;
int *Grade;
int GradeCount;
} Enroll_Database;
Function...
void addGrade(Enroll_Database *Enroll)
{
int i = 0, j = 0, b, Course_Num, Student_Num, Grade;
printf("Enter Course Number: ");
scanf("%d", &Course_Num);
printf("Enter Student ID: ");
scanf("%d", &Student_Num);
/* For loop that traverses through the Enroll array until until it encounters
nothing in the Course ID */
for(i = 0; Enroll[i].Course_ID != 0; i++)
{
/* if the Student Number and the Course Number are equal to their
appropriate Enroll element, then ask user to input Grade */
if(Enroll[i].Student_ID == Student_Num && Enroll[i].Course_ID == Course_Num)
{
printf("Enter Grade: ");
scanf("%d", &Grade);
if(Enroll[i].GradeCount == 0)
{
Enroll->Grade = (int *) malloc(sizeof(int));
Enroll[i].Grade[Enroll[i].GradeCount] = Grade; //core dumps
Enroll[i].GradeCount++;
}
else
{
Enroll->Grade = (int *) realloc(Enroll->Grade, sizeof(int));
Enroll[i].Grade[Enroll[i].GradeCount] = Grade; //core dumps
Enroll[i].GradeCount++;
}
}
}
}
I've ran multiple checks and the core dump occurs after I malloc/realloc and assign user input to the grade value in the enroll structure.
I'd greatly appreciate any help, and I'm sorry again if my code is unreadable or I formatted wrong. Thanks!
This only allocates space for one element,. and also it reallocates the wrong pointer:
Enroll->Grade = (int *) realloc(Enroll->Grade, sizeof(int));
It could be fixed like this:
Enroll[i].Grade = realloc(Enroll[i].Grade, sizeof(int) * (Enroll[i].GradeCount + 1));
Remembering that X->Y, (*X).Y, and X[0].Y all mean the same thing: your original version actually reallocated Enroll[0].Grade, instead of Enroll[i].Grade.
(The rest of this answer is some possible style improvement suggestions:)
To avoid this sort of error, I'd personally write just after the scanf:
Enroll_Database *found = &Enroll[i];
and then use found-> everywhere instead of Enroll[i]. Alternatively I'd consider having a separate function to actually add the grade (which is called once the database entry has been found).
Now, if you initialize Enroll[i].Grade to NULL when setting up the database, you actually wouldn't need this if...else statement. Since the behaviour of realloc(NULL, X) is the same as malloc(X), the same code would handle both situations.
Note that in C you should not cast the value returned by malloc and friends.
Another thing to bear in mind is that the style X = realloc(X, ... does not allow you to recover from allocation failure. To write robust code you need to take some sensible action when a malloc-family function returns NULL; this could be as simple as printing a message and calling exit.