What is wrong with how I'm dynamically allocating space for this 2d array?

Question

I'm trying to create a 2D array that will store be able to store each character of a .txt file as an element in the 2D array.

How do I dynamically allocate space for it?

This what I've done so far to malloc it. (this was copied of GeeksForGeeks)

char *arr[rownum2];

for (i = 0; i < rownum2; i++) {
    arr[i] = (char *)malloc(colnum * sizeof(char));

However, I think this is the source of serious memory related issues later on in my program, and I've also been told some parts of this are unnecessary.

Can I please get the most suitable way to dynamically allocate memory for the 2D array in this specific scenario?

Answer 1

The code you have posted is 'OK', so long as you remember to call free() on the allocated memory, later in your code, like this:

for (i=0;i<rownum2;i++) free(arr[i]);

...and I've also been told some parts of this are unnecessary.

The explicit cast is unnecessary, so, instead of:

arr[i] =  (char *)malloc(colnum*sizeof(char));

just use:

arr[i] =  malloc(colnum*sizeof(char));

The sizeof(char) is also, strictly speaking, unnecessary (char will always have a size of 1) but you can leave that, for clarity.

Answer 2

Technically, it's not a 2D array, but an array of arrays. The difference is, you can't make 2D array with lines of different size, but you can do it with your array of arrays.

If you don't need it, you can allocate rownum2*colnum elements and access each element as arr[x+colnum*y] (it's used often because all data are kept in one place, decreasing CPU cache load and some system inner needs for storing each pointer of each allocated chunk).

Also, even array of lines of different sizes can be placed into 1D array and accessed like 2D (at least, if they do not change size or even RO). You can allocate char body[total_size], read the whole array, allocate char* arr[rownum2] and set each arr[i]=body+line_beginning_offset.

BTW don't forget there are not actual C strings because they are not null-terminated. You'll need an additional column for null-term. If you store ASCII art, 2D array is a very good solution.

Answer 3

The only serious problem I see in your code is that you are casting the returned value of malloc(3), and probably you have forgotten to #include <stdlib.h> also (this is a dangerous cocktail), and this way, you are destroying the returned value of the call with the cast you put before malloc(3). Let me explain:

• First, you have (or haven't, but I have to guess) a 64bit architecture (as it is common today) and pointers are 64bit wide in your system, while int integers are 32bit wide.
• You have probably forgotten to #include <stdlib.h> in your code (which is something I have to guess also), so the compiler is assuming that malloc(3) is actually a function returning int (this is legacy in C, if you don't provide a prototype for a function external to the compilation unit), so the compiler is generating code to get just a 32 bit value from the malloc(3) function, and not the 64bit pointer that (probably, but I have to guess also) malloc(3) actually returns.
• You are casting that int 32bit value (already incorrect) to a 64bit pointer (far more incorrect, but I have to guess...), making any warning about type conversions between integer values and pointers to dissapear, and be silenced when you put the cast (the compiler assumes that, as a wise programmer you are, you have put the cast there on purpose, and that you know what you are doing)
• The first (undefined behaviour) returned value is being (undefined behaviour) just cut to 32 bit, and then converted (from int to char *, with more undefined behaviour) to be used in your code. This makes the original pointer returned from malloc(3) to be completely different value when reinterpreted and cast to (char *). This makes your pointers to point to a different place, and break your program on execution.

Your code should be something like (again, a snippet has to be used, as your code is not complete):

#include <stdlib.h> /* for malloc() */

/* ... */

char *arr[rownum2];

for (i = 0; i < rownum2; i++) {
    arr[i] = malloc(colnum); /* sizeof(char) is always 1 */

I need finally to do you a recommendation:

Please, read (and follow) the how to create a minimal, verifiable example page, as your probable missing #include error, is something I had to guess.... Posting snippets of code makes many times your mistakes to go away, and we have to guess what can be happening here. This is the most important thing you have to learn from this answer. Post complete, compilable and verifiable code (that is, code that you can check fails, before posting, not a snippet you selected where you guess the problem can be). The code you posted does allow nobody to verify why it can be failing, because it must be completed (and repaired, probably) to make it executable.