CODEWITHSUNDEEP
cmemorymalloc
SwimMaster
SwimMaster

Reputation: 381

Invalid initialization of dynamic array leads to read and write errors

I'm trying to malloc an array of structs. 

typedef struct {
    long int val;
    long int time;
    long int last_used;
} pair;

so in my main I have

pair **fifoVM = (pair **) malloc(sizeof(pair *) * framecount);
pair **fifop1VM = (pair **) malloc(sizeof(pair *) * framecount + 1);
pair **fifop2VM = (pair **) malloc(sizeof(pair *) * framecount + 2);
pair **LRUVM = (pair **) malloc(sizeof(pair *) * framecount);

and I initialize all of the pairs using 

void init(pair **frames, int size) {
    for (int i = 0; i < size; i++) {
        frames[i] = (pair *) malloc(sizeof(pair));
        frames[i]->val = -1;
        frames[i]->last_used = TIME_VAL;
        frames[i]->time = TIME_VAL++;
    }
}

But by the time I try to deallocate it, I get a corruption error from Valgrind.

I initially thought that the problem was using a pair* in the array but it still didn't work with just pair. I also thought it might be the pair going out of scope when init() returns but that also inst true because it would only deallocate the variable containing the pointer.

Also for some weird reason, LRUVM is the only array to crash, even though it's the last one. 

#include <stdio.h>
#include <limits.h>
#include <stdlib.h>
#include <time.h>

//since time.h only has millisecond resolution,
//I need to simulate time
int TIME_VAL = 0;

typedef struct {
    long int val;
    long int time;
    long int last_used;
} pair;

//Allocate the pairs for a given array
void init(pair **frames, int size) {
    //iterate through array
    for (int i = 0; i < size; i++) {
        //allocate memory and assign
        frames[i] = (pair *) malloc(sizeof(pair));
        frames[i]->val = -1;
        frames[i]->last_used = TIME_VAL;
        frames[i]->time = TIME_VAL++;
    }
}

int main(int argc, char **argv) {
    //Command line arguments
    int framecount = atoi(argv[1]);
    int x = atoi(argv[2]);
    int NUM_ACCESSES = atoi(argv[3]);
    int NUM_ITERATIONS = atoi(argv[4]);

    for (int i = 0; i < NUM_ITERATIONS; i++) {

        //Allocate Arrays
        pair **fifoVM = (pair **) malloc(sizeof(pair *) * framecount);
        pair **fifop1VM = (pair **) malloc(sizeof(pair *) * framecount + 1);
        pair **fifop2VM = (pair **) malloc(sizeof(pair *) * framecount + 2);
        pair **LRUVM = (pair **) malloc(sizeof(pair *) * framecount);

        //initialize all of the pairs in the arrays
        init(fifoVM, framecount);
        init(fifop1VM, framecount + 1);
        init(fifop2VM, framecount + 2);
        init(LRUVM, framecount);

        //deallocate arrays
        freeList(fifoVM, framecount);
        freeList(fifop1VM, framecount + 1);
        freeList(fifop2VM, framecount + 2);
        freeList(LRUVM, framecount);
    }
}

void freeList(pair **vm, int framecount) {
    for (int i = 0; i < framecount; i++) {
        free(vm[i]);
    }
    free(vm);
}

Upvotes: 1

Views: 45

Answers (1)

chqrlie
chqrlie

Reputation: 144951

Some of the allocation sizes are not computed correctly: malloc(sizeof(pair *) * framecount + 1) should be:

malloc(sizeof(pair *) * (framecount + 1))

Note that your data structure seem to have an indirection for no good reason. Why not allocate arrays of structures instead of arrays of pointers to structures allocated individually?

Here is a simpified version:

#include <stdio.h>
#include <limits.h>
#include <stdlib.h>
#include <time.h>

//since time.h only has millisecond resolution,
//I need to simulate time
int TIME_VAL = 0;

typedef struct {
    long int val;
    long int time;
    long int last_used;
} pair;

//Allocate the pairs for a given array
void init(pair *frames, int size) {
    for (int i = 0; i < size; i++) {
        frames[i].val = -1;
        frames[i].last_used = TIME_VAL;
        frames[i].time = TIME_VAL++;
    }
}

int main(int argc, char **argv) {
    //Command line arguments
    if (argc < 5) return 1;

    int framecount = atoi(argv[1]);
    int x = atoi(argv[2]);
    int num_accesses = atoi(argv[3]);
    int num_iterations = atoi(argv[4]);

    for (int i = 0; i < num_iterations; i++) {
        //Allocate Arrays
        pair *fifoVM = calloc(sizeof(pair), framecount);
        pair *fifop1VM = calloc(sizeof(pair), framecount + 1);
        pair *fifop2VM = calloc(sizeof(pair), framecount + 2);
        pair *LRUVM = calloc(sizeof(pair), framecount);

        if (fifoVM && fifop1VM && fifop2VM && LRUVM) {
            //initialize all of the pairs in the arrays
            init(fifoVM, framecount);
            init(fifop1VM, framecount + 1);
            init(fifop2VM, framecount + 2);
            init(LRUVM, framecount);

            //...
        }
        //deallocate arrays
        free(fifoVM);
        free(fifop1VM);
        free(fifop2VM);
        free(LRUVM);
    }
}

Upvotes: 1

Related Questions