1

I've looked around and found answers on how to allocate memory properly for a 2D array with pointer notation, and I've found how to properly pass a 2d pointer array to a function, but I can't seem to combine the two steps.

So logically what I'm trying to do is allocate an array of pointers to structs. What appears to be failing is trying to allocate memory

my code thus far:

typedef struct {
int tax, savings;
float salary;
} employee;

int readRecordFile(char* filename, Record*** array);

int main(void) {

employee** array;
int size;
char* file = "money.csv";
size = readRecordFile(file, &array);

FILE* fptr;
fptr = fopen(file, "r");


//Read the file into the the array 
for (int i = 0; i < size; i++) {
    fscanf(fptr, "%d,%d,%f", array[i]->tax, array[i]->savings, array[i]->salary);
}
fclose(fptr);


for (int i = 0; i < size; i++) {
    printf("%d, %d, %f\n", array[i]->tax, array[i]->savings, array[i]->salary);
}
return 0;
}

int readRecordFile(char* filename, employee*** array) {
//Function to open the file, malloc and initialize 2d arrays

//Open the file
FILE* fileptr;
fileptr = fopen(filename, "r");
if (fileptr == NULL) {
    printf("Failed to open file");
    exit(-1);
}

//Read first line of file (the size) and store to an int
int n;
fscanf(fileptr, "%d", &n);

//Initial malloc for the array of pointers
**array = malloc(n * sizeof(employee*)); //This is the line that throws the exception

//Malloc for each pointer in the array of pointers
for (int i = 0; i < n; i++) {
    *(array+i) = malloc(sizeof(employee));
}


//Close the file and return the size of the file
fclose(fileptr);

return n;
}

I have tried building a separate struct pointer within the function, then setting the regular pointer to it

    //Initial malloc for the array of pointers
employee **tester = malloc(n * sizeof(employee*));

//Malloc for each pointer in the array of pointers
for (int i = 0; i < n; i++) {
    *(tester+i) = malloc(sizeof(employee));
}

array = tester;

This method appears to fix the malloc issues, but the data printing in main fails.

Jimbo
  • 11
  • 1
  • Possible duplicate: [Correctly allocating multi-dimensional arrays](https://stackoverflow.com/questions/42094465/correctly-allocating-multi-dimensional-arrays) – Lundin Sep 26 '19 at 06:19

2 Answers2

0

You are allocating the 2D array of struct correctly.

But you are only reading into a 1D array of struct. (e.g. array[i]->tax)

To read a 2D array of struct you need 2 for loops

for (int i = 0; i < size; i++) {
    for (int j=0; j< size; j++) { // in your case you have a size*size array, 
        fscanf(fptr, "%d,%d,%f", array[i][j]->tax, array[i][j]->savings, array[i][j]->salary);
     }
}

You need to decide whether you need a 1D or a 2D array.

Rishikesh Raje
  • 8,556
  • 2
  • 16
  • 31
0

employee **array; is NOT an array, it is a single-pointer. (pointer to pointer to employee).

What is the value of size when you call:

for (int i = 0; i < size; i++) 

size is indeterminate (its value is whatever garbage value happens to be in memory at program startup). You invoke Undefined Behavior attempting to access the indeterminate value. size must be set before it is used (same for all variables)

At this same point, array is an uninitialized pointer, attempting to access an indeterminate address is just as bad. (Remember, a pointer is simply a normal variable that holds the address of something else as its value). Since the address held by array has not yet been assigned and no storage has been allocated for any employee struct, you cannot do:

fscanf(fptr, "%d,%d,%f", array[i]->tax, array[i]->savings, array[i]->salary);

Before you use array, you must:

  1. allocate some number of pointers and assign the start of the memory block holding the allocated pointers to array, and then
  2. allocate storage for each of your struct and assign the starting address for each block of storage holding a struct to one of the pointers array[i].

For example to initially declare 64 pointers, you could do:

array = malloc (64 * sizeof *array);

(note: using the dereferenced pointer (sizeof *array) will always result in the correct type-size rather than trying to match the type, e.g. sizeof (employee*))

ALWAYS validate every allocation, e.g.

if (array == NULL) {            /* if the pointer returned by malloc is NULL */
    perror ("malloc-array");    /* issue diagnostic (perror is simple) */
    exit (EXIT_FAILURE);        /* handle error */
}

Now how to allocate for each struct? (the same way). But, an approach that is much easier for reading, allocating and validating is to use a temporary struct with automatic storage and read and fill the temporary struct. If your read succeeds, then allocate storage for your struct, assign the starting address for that block of memory to array[i] and then assign the contents of the temporary struct to your new block of memory. For example (after your fix size initialization):

for (int i = 0; i < size; i++) {
    employee tmp = { .tax = 0 };    /* declare temporary struct, initialized zero */
    /* read values into temporary struct / VALIDATE every input */
    if (fscanf (fptr, "%d,%d,%f", tmp.tax, tmp.savings, tmp.salary) == 3) {
        array[i] = malloc (sizeof *array[i]);   /* allocate for struct */
        if (!array[i]) {                        /* validate allocation */
            perror ("malloc-array[i]");
            exit (EXIT_FAILURE); 
        }
        array[i] = tmp;             /* assign temp struct to allocated block */
    }
}

Now when you are done with using your "array", don't forget to free the memory allocated. For example, assuming you validly filled size, then you could do:

for (int i = 0; i < size; i++)
    free (array[i]);            /* free each allocated struct */
free (array);                   /* free pointers */

There are probably additional errors in your posted code (it is in somewhat of a jumbled order), but these are the most immediate errors you are facing. Make the changes and test further. Make sure all variables are properly initialized before their use. Make sure you have allocated storage for each pointer you will use and then allocate for each struct assign the address for the struct storage to one of your allocated pointers. If you hit another road block, either edit this question (or preferably ask a new question) noting the specific problem you are stuck on.

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85