EXC_BAD_ACCESS code 2 address, testq %rcx, -0x8(%rcx)

Question

here is a part of my code:

    typedef enum {FREE, OCCUPIED} _Status;

typedef struct {
    char spec[SIZE];
    _Status *bed[DAY];
    int b;
} _Unit;

typedef struct {
    char name[SIZE +1];
    _Unit *unit;
    int u;
} _Hospital;

void error(char *err){
    fprintf(stderr, "error: %s\n", err);
    exit(EXIT_FAILURE);
}
FILE *fileOpen (char *f_name){
    FILE *f;
    if ((f = fopen(f_name, "r")) == NULL){
        error("file");
    }

    return f;
}

_Hospital * loadHospital(FILE *f, int *n){
    int i, j, k, w;
    _Hospital *H;
    H = (_Hospital *) malloc (*n * sizeof(_Hospital));
    for (i = 0; i < *n; i++){
        H[i].u = 0;
    }
    if (H == NULL)
        error ("memory");

    for (i = 0; i < *n; i++){
        H[i].unit = (_Unit *) malloc (1 * sizeof(_Unit));
        if (H[i].unit == NULL)
            error ("memory");
        for (j = 0; j < DAY; j++){
            H[i].unit[0].bed[j] = (_Status *) malloc (1 * sizeof(_Status));
            if (H[i].unit[0].bed[j] == NULL)
                error ("memory");
        }
    }

    char tmp_name[SIZE];
    char tmp_unit[SIZE];
    int tmp_bed;
    int h, counter_h = 0;
    while (fscanf(f, "%s %s %d", tmp_name, tmp_unit, &tmp_bed) != EOF){
        h = -1;
        for (i = 0; i < *n && h == -1; i++){
            if (strcmp(H[i].name, tmp_name) == 0)
                h = i;
        }
        if (h == -1){
            strcpy(H[counter_h].name, tmp_name);
            strcpy(H[counter_h].unit[H[counter_h].u].spec, tmp_unit);
            H[counter_h].unit[H[counter_h].u].b = tmp_bed;
            for (k = 0; k < DAY; k++){
                H[counter_h].unit[H[counter_h].u].bed[k] =
                        (_Status *) realloc(H[counter_h].unit[H[counter_h].u].bed[k], H[counter_h].unit[H[counter_h].u].b * sizeof(_Status));
                if (H[counter_h].unit[H[counter_h].u].bed[k] == NULL) error("memory");
                for (w = 0; w < H[counter_h].unit[H[counter_h].u].b; w++){
                    H[counter_h].unit[H[counter_h].u].bed[k][w] = FREE;
                }
            }
            H[counter_h].u++;
            counter_h++;

        } else {
            strcpy(H[h].unit[H[h].u].spec, tmp_unit);
            for (k = 0; k < DAY; k++) {
                H[h].unit[H[h].u].bed[k] =
                        (_Status *) realloc(H[h].unit[H[h].u].bed[k], H[h].unit[H[h].u].b * sizeof(_Status));
                if (H[h].unit[H[h].u].bed[k] == NULL) error("memory");
                for (w = 0; w < H[h].unit[H[h].u].b; w++) {
                    H[h].unit[H[h].u].bed[k][w] = FREE;
                }
            }

            H[h].u++;
        }
    }

    *n = counter_h;
    return H;



}

//void reserveBed (_Hospital **H, int n);

//void removeBed (_Hospital **H, int n);

void freeMemory (_Hospital *H, int n){
    for (int i = 0; i < n; i++){
        for (int j = 0; j < H[i].u; j++) {
            for (int k = 0; k < DAY; k++)
                free(H[i].unit[j].bed[k]);
        }

        free(H[i].unit);
    }

    free(H);
}

int main() {


    FILE *f1_ptr;
    f1_ptr = fileOpen("Hospitals.txt");

    int n;
    fscanf (f1_ptr, "%d", &n);

    _Hospital *hospital;
    hospital = loadHospital(f1_ptr, &n);
    fclose (f1_ptr);

    freeMemory(hospital, n);

    return EXIT_SUCCESS;
}

When I try and debug it, it doesn't even start and CLion shows me the error EXC BAD ACCESS code 2. It then redirects me to some assembly code I don't understand. I just started on dynamic memory allocation, so I'm probably not aware of some horrible mistake I may have done. Being unable to debug it, I didn't manage to spot it though.

I'm not sure I posted the question in the right way, I didn't know what to include. Thank you for any suggestion.

Answer 1

Problems

Here are a couple of issues I've found with your code so far:

1. Don't pass the number of hospitals loadHospital(FILE *f, int *n) as a pointer. Its messy, and prone to cause mistakes.
2. You only allocate one unit per hospital initially. And you also don't assign the field u in the hospital struct, which you use later as an indicator of how many units to free. This will cause undefined behaviour.
3. I'm not sure what you're doing when you're scanning in the names, units, and number of temporary beds. You're assiging h the value -1, but you seem to be trying to break from the for loop right after if it finds a hospital with the same name as the temporary one. The thing is, you never initialized the memory for the name in the hospital. So it might not even be NULL terminated, which messes with the strcmp. It should still get through because tmp_name is correctly terminated, but it's incorrect nontheless.
4. You perform a strcpy in order to set the name of the hospital, but don't make sure that the tmp_name is smaller than the buffer size. You should use strncpy and specify a maximum size.
5. You attempt to assign to a unit using index H[counter_h].u as the indexing value. You do realize that you only have one unit allocated per hospital right? And that the field u in each hospital was never set. So it could be any value. This will likely be causing segfaults that you're picking up in your debugger.

Recommendations

I recommend you do the following:

1. Remove all the parsing code for now, and make sure your allocator and deallocator steps are working propely first.
2. Bring back in the parser, and make sure that you're assigning to fields that exist, and accessing ones that were properly initialized.

---

I've already cleaned up the allocation / deallocation step. But you're going to need to adjust it if you expect that there should be more than one unit per hospital. I've also not re-written your parsing step. You can do that.

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


#define SIZE 32
#define DAYS 16

typedef enum {
    FREE,
    OCCUPIED
} status_t;

typedef struct {
    char      spec[SIZE];
    status_t* beds[DAYS];   // Array of pointers
    int       b;
} unit_t;

typedef struct {
    char      name[SIZE];
    unit_t*   unit_p;
    int       u;
} hospital_t;


// Notes:
// There are 'count' hospitals
// Each hospital allocates one unit, with 'days' statuses allocatd within
hospital_t* read_hospitals (FILE *file, int count)
{
    int i, j, k, w;
    hospital_t *hospital_p = NULL;

    // 1. Allocate 'count' hospital structs
    hospital_p = (hospital_t *)malloc(count * sizeof(hospital_t));

    // 2. Check for NULL **AFTER** allocating and before any other assignments
    if (NULL == hospital_p) {
        fprintf(stderr, "Unable to allocate %lu bytes!\n", sizeof(hospital_t));
        exit(EXIT_FAILURE);
    }

    // 3. Now begin allocating each individual unit
    for (i = 0; i < count; i++) {
        unit_t* unit_p = malloc(sizeof(unit_t));
        if (NULL == unit_p) {
            fprintf(stderr, "Unable to allocate %lu bytes!\n", sizeof(unit_t));
            exit(EXIT_FAILURE);
        }
        for (j = 0; j < DAYS; j++) {
            status_t* status_p = malloc(sizeof(status_t));
            if (NULL == status_p) {
                fprintf(stderr, "Unable to allocate %lu bytes!\n", sizeof(status_t));
            }
            unit_p->beds[j] = status_p;
        }
        hospital_p[i].unit_p = unit_p;
        hospital_p[i].u      = 1;
    }

    // TODO: Your file input parsing here

    return hospital_p;
}

void free_hospitals (hospital_t *hospitals, int count)
{
    int i, j;

    // Don't free the hospitals themselves, they are a contiguous array
    for (i = 0; i < count; i++) {

        // Extract the unit for the hospital at index i
        unit_t* unit_p = hospitals[i].unit_p;

        // Free the statuses that were allocated to the unit
        for (j = 0; j < DAYS; j++) {
            free(unit_p->beds[j]);
        }

        // Free the unit itself
        free(unit_p);
    }

    free(hospitals);
}

int main (void)
{
    int hospital_count = 15;

    // Allocate N hospitals
    hospital_t *hospitals = read_hospitals(NULL, hospital_count);

    // Free the hospitals
    free(hospitals);

    return EXIT_SUCCESS;
}