creallocmemory-reallocation

Why is realloc giving me inconsistent behaviour?

I am currently taking a procedural programming course at my school. We are using C with C99 standard. I discussed this with my instructor and I cannot understand why realloc() is working for his machine, but it is not working for mine.

The goal of this program is to parse a text file students.txt that has students' name and their GPA formatted like this:

Mary 4.0
Jack 2.45
John 3.9
Jane 3.8
Mike 3.125

I have a function that resizes my dynamically allocated array, and when I use realloc the debugger in my CLion IDE, it gave me SIGABRT.

I tried using an online compiler and I get realloc(): invalid next size.

I have been trying to debug this all weekend and I can't find the answer and I need help.

My code is currently looking like this

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

#define INITIAL_SIZE 4
#define BUFFER_SIZE 512
#define GRADE_CUTOFF 3.9

// ERROR CODES
#define FILE_OPEN_ERROR 1
#define MEMORY_ALLOCATION_ERROR 2

struct student {
    double gpa;
    char *name;
};

struct student *resizeAllocationIfNeeded(struct student *listOfStudents,
        unsigned int studentCount, size_t *currentSize) {

    if (studentCount <= *currentSize) {
        return listOfStudents;
    }

    *currentSize *= 2;
    struct student *resizedList = (struct student *) realloc(listOfStudents, *currentSize * sizeof(struct student));
    if (resizedList == NULL) {
        perror("Failed to allocate memory");
        exit(MEMORY_ALLOCATION_ERROR);
    }
    return resizedList;
}

size_t getNamesAndGrades(FILE *file, struct student *listOfStudents, size_t size) {
    unsigned int studentCount = 0;
    char buffer[BUFFER_SIZE];

    while(fscanf(file, "%s %lf", buffer, &listOfStudents[studentCount].gpa) > 0) {
        listOfStudents[studentCount].name = strdup(buffer);
        studentCount++;
        listOfStudents = resizeAllocationIfNeeded(listOfStudents, studentCount, &size);
    }

    return studentCount;
}

void swapStudents(struct student *listOfStudents, int x, int y) {
    struct student temp = listOfStudents[x];
    listOfStudents[x] = listOfStudents[y];
    listOfStudents[y] = temp;
}

void sortStudentsByGPA(struct student *listOfStudents, unsigned int studentCount) {
    for (int i = 0; i < studentCount; i++) {
        for (int j = 0; j < studentCount - i - 1; j++) {
            if (listOfStudents[j].gpa < listOfStudents[j + 1].gpa) {
                swapStudents(listOfStudents, j, j + 1);
            }
        }
    }
}

void printStudentAndGPA(struct student *listOfStudents, unsigned int studentCount) {
    for (int i = 0; i < studentCount; i++) {
        if (listOfStudents[i].gpa > GRADE_CUTOFF) {
            printf("%s %lf\n", listOfStudents[i].name, listOfStudents[i].gpa);
        }
        free(listOfStudents[i].name);
    }
}

void topStudents(char *fileName) {
    FILE *file = fopen(fileName, "r");

    if (!file) {
        perror("Could not open file for reading");
        exit(FILE_OPEN_ERROR);
    }

    struct student *listOfStudents = (struct student *) malloc(INITIAL_SIZE * sizeof(struct student));

    if (listOfStudents == NULL) {
        perror("Failed to allocate memory");
        exit(MEMORY_ALLOCATION_ERROR);
    }

    unsigned int studentCount = getNamesAndGrades(file, listOfStudents, INITIAL_SIZE);
    sortStudentsByGPA(listOfStudents, studentCount);
    printStudentAndGPA(listOfStudents, studentCount);
    free(listOfStudents);
}

int main() {
    topStudents("students.txt");
    return 0;
}
Solution

  • There are two errors in your code: one is just a typo, the other is a more serious logical error.

    First, you are reallocating too late, because of the condition in resizeAllocationIfNeeded(). When studentCount == currentSize, this doesn't resize (even though it should), which makes you overflow the array of students and causes problems.

    You can change the condition to fix this:

    if (studentCount < *currentSize) {
    return listOfStudents;
}

    Apart from the above, your main error is in getNamesAndGrades(), where you are reallocating memory and assigning the new pointers to a local variable. You then use that variable in topStudents() as if it was updated. This will of course not work, as the initial pointer passed by topStudents() becomes invalid after the first realloc() and memory is irrevocably lost when getNamesAndGrades() returns.

    You should either pass a pointer to the student array, or better just make the function create the array for you.

    Here's a solution, renaming getNamesAndGrades to getStudents:

    struct student *getStudents(FILE *file, unsigned int *studentCount) {
    char buffer[BUFFER_SIZE];
    struct student *listOfStudents;
    size_t size = INITIAL_SIZE;

    *studentCount = 0;
    listOfStudents = malloc(size * sizeof(struct student));
    
    if (listOfStudents == NULL) {
        perror("Failed to allocate memory");
        exit(MEMORY_ALLOCATION_ERROR);
    }

    while(fscanf(file, "%511s %lf", buffer, &listOfStudents[*studentCount].gpa) == 2) {
        listOfStudents[*studentCount].name = strdup(buffer);
        (*studentCount)++;
        listOfStudents = resizeAllocationIfNeeded(listOfStudents, *studentCount, &size);
    }

    return listOfStudents;
}

// ...

void topStudents(char *fileName) {
    FILE *file = fopen(fileName, "r");

    if (!file) {
        perror("Could not open file for reading");
        exit(FILE_OPEN_ERROR);
    }

    unsigned int studentCount;
    struct student *listOfStudents = getStudents(file, &studentCount);

    sortStudentsByGPA(listOfStudents, studentCount);
    printStudentAndGPA(listOfStudents, studentCount);
    free(listOfStudents);
}

int main() {
    topStudents("students.txt");
    return 0;
}

    Additional notes: