1

I am trying to create a 2d array which each element points to a struct to store values from a CSV file. I am not sure if I am creating the struct or defining the 2d array right though.

Below is my struct and how I allocate the memory. There will always be 35 columns but the rows could grow quite large. When I run this with int test =29000 (or 29000 rows), it works and stores all values in a 2d array I can access by [row][col]. When I assign test with anything greater than 29000 it seg faults without even going into the allocation of the memeory it just segfaults at struct node* arrayofnodes[test][35];

I am very confused as to why it works with 29000 but not with 30000. Also, if anyone has any suggestions on how to malloc the array on one line instead of having to go into 2 for loops, I would be very happy to learn. Also I was wondering If I should use a typedef struct here or not.

Thanks

 struct node {
  char* value;
};

int test=30000;
struct node* arrayofnodes[test][35];
for (int p=0; p < test; p++){
   for (int j=0; j < 35; j++){
      arrayofnodes[p][j] = malloc(sizeof(arrayofnodes));
   }
}//this works and i can access a certain element by arrayofnodes[row][col].value;
ggorlen
  • 44,755
  • 7
  • 76
  • 106
  • 2
    Note: `sizeof(arrayofnodes)` is the size of `test * 35` pointers - and code is allocating that `test * 35` times. – chux - Reinstate Monica Mar 12 '19 at 03:51
  • @Antii Haapala, there is still more to this question that has not been answered by the posts marked as duplicate. If you would, please, re-open the question so I can add some additional insight to help cover it. Thanks. – Gabriel Staples Mar 28 '19 at 18:51

3 Answers3

1

You're allocating a huge amount of memory here: printf("%lu\n", sizeof(arrayofnodes)); reports that arrayofnodes has a size of 30000 * 35 * 8 = 8400000 bytes, making for a total memory allocation of 30000 * 35 * 8400000 = 8820000000000 bytes in your loop.

You might have meant *arrayofnodes[p][j]--allocate space for a pointer to a struct at row p and column j. Note that this doesn't include space for the memory block pointed to by the char *, which will also need to be malloced.

Also, remember that while the heap has plenty of space, the stack doesn't, and 8 * 35 * 30000 is likely more than a stack frame can handle.

Consider avoiding a struct if there's only one field. Using a plain old char * would avoid an extra layer of indirection.

If you'd like to maximize space efficiency at the expense of speed, you might try reading your text file in once to build an array of string sizes for each cell, then malloc everything accordingly for a second pass. You could use a char ***csv_data array containing cells allocated to size csv_cell_size[row][col] or similar.

Flattening your array by a dimension and using an offset to find a particular row/column is another option. Long story short, there are more than a few ways to manage this, with a lot depending on your data and how you plan to use it.

Also, don't forget to free() all your memory when you're done with it.

ggorlen
  • 44,755
  • 7
  • 76
  • 106
1

The problem is likely that you're allocating to much on the stack. arrayofnodes will be an array of 30,000*35=1,050,000 pointers, each of which is probably 8 bytes. So you're trying to allocate an array of 8MB on the stack, and 8MB is the default limit for stack size.

This code segfaulted for me:

#include <stdio.h>

int main() {
    struct node {
        char* value;
    };

    int test=30000;
    struct node* arrayofnodes[test][35];
    printf("%d\n", sizeof(arrayofnodes));
}

Also, sizeof(arrayofnodes) is wrong. If the code snippet above wouldn't have segfaulted, it would have printed 8400000. Instead, sizeof struct node or sizeof arrayofnodes[0][0] would give the right result.

A solution could be something like this:

struct node ** array[35];
for(int i=0; i<35; i++) {
    array[i] = malloc(test * sizeof array[0]);
    for(int j=0; j<test; j++) array[i][j] = malloc(sizeof (struct node));
}
klutt
  • 30,332
  • 17
  • 55
  • 95
  • I've upvoted your answer, but you incorrectly said: `If the code snippet above wouldn't have segfaulted, it would have printed 22400000`. It would have printed 35*30000*8 = `8400000`. I debated if I should edit your answer but decided to post this as a comment so you can fix it instead. I've verified it though an actual program too, where setting `test` to 10000 runs (rather than having a `Segmentation fault (core dumped)` error, and it prints `2800000`. 2800000 * 3 = 8400000 as well. I'm really not sure where you got the `22400000` number from. – Gabriel Staples Mar 28 '19 at 18:08
  • Also, the 2nd part of your answer (your recommended solution) is also incorrect (and hard to read). For your 2nd `malloc()`, you should be allocating memory for the size of the `struct node`. Using `malloc(sizeof array[0][0])`, however, is **wrong**, as it is mistakenly allocating memory for the size of the `struct node *` instead. Valid options include all 3 of the following: `sizeof(*array[0][0])`, or `sizeof(array[0][0][0])`, or `sizeof(struct node)` (this last one is the RECOMMENDED option, since it's the most readable and hardest to mess up). – Gabriel Staples Mar 28 '19 at 18:38
  • ^^ sorry, didn't mean for that word to be all caps. – Gabriel Staples Mar 28 '19 at 18:44
0

Your array declaration is being allocated on the stack, the default stack limit is 8MB. When test is 29000 the stack allocation is within the limit (29000 * 35 * 8 = 7.7MB), when you change test to 30000 you are exceeding the stack limit (30000 * 35 * 8 = 8.01MB) which results int the seg fault.

You can get past this by allocating the array on the heap using malloc, just remember to free what you allocate.

#include <stdio.h>
#include <malloc.h>

struct node {
        char *value;
};
int rows = 30000;
int cols = 35;

struct node **arrayofnodes;

int main() {
        arrayofnodes = (struct node **)malloc(rows * sizeof(struct node*));
        for (int row = 0; row < rows; row++) {
                arrayofnodes[row] = (struct node *)malloc(cols * sizeof(struct node*));
        }

        // Use arrayofnodes

        // Free allocations after use...
}
Chris Taylor
  • 52,623
  • 10
  • 78
  • 89