1

I am trying to create a two dimensional array based on two pre-calculated values (here len1 and len2 are const, but they come from another function). When I run this I get segmentation fault. I am very new to C, this is my first task. Can not figure it out from Guides nor SO, anybody around to help me out?

I suppose the dynamic creation of the two dimensional arrays is wrong. But can't find a good example that would work ..

int main() {
    
    int y, x;
    
    int my_val = 10; // dynamnic value calculated by another func
    int len1 = 3; // dynamnic value calculated by another func
    int len2 = 3; // dynamnic value calculated by another func
    
    int cols = len1 + 1;
    int rows = len2 + 1;
    
    int **twodarr = (int **)malloc(rows * cols * sizeof(int));
    
    for (x = 1; x < cols; x++) {
        for (y = 1; y < rows; y++) {
            twodarr[y][x] = my_val;
        }
    }
    
    return 0;
}
Stefan
  • 828
  • 12
  • 24

4 Answers4

2

You have to allocate each rows

// allocation of cols
int **twodarr = (int **)malloc(cols * sizeof(int*));// note it is sizeof(int*)

// allocation each rows (in each cols)
for (x = 0; x < cols; x++) {
    twodarr[x] = (int *)malloc(rows * sizeof(int));
}
ebuckis
  • 27
  • 3
  • In C, casting mallocs is not recommended. – Yunnosch Jun 29 '21 at 09:47
  • 1
    The larger problem here isn't the cosmetic cast, but the fragmented slow pointer look-up table which serves no purpose but to create complexity and lag, for absolutely nothing gained. See [Correctly allocating multi-dimensional arrays](https://stackoverflow.com/questions/42094465/correctly-allocating-multi-dimensional-arrays). – Lundin Jun 29 '21 at 09:53
  • The malloc casting .. again .. So if you don't cast malloc you wont be able to port your code to C++ as the cast is required here. There is nothing inherently wrong in the cast in C, it used to **not** give you a warning about missing stdlib.h when you did the cast, but that isn't the case in newer compilers, and in C void pointers are automatically and safely promoted. At least that is what I have gathered from this forum. – Sorenp Jun 29 '21 at 10:13
2

The problem is that int **twodarr cannot be used for 2D arrays, it has no relation what-so-ever to them. You need to swap it for a pointer to a 2D array. Or more conveniently, a pointer to a 1D array - a pointer to a row in this case, assuming [rows][cols].

Also, arrays in C start at index 0.

Code with bug fixes & a simple print example:

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

int main() {
    
    int my_val = 10; // dynamnic value calculated by another func
    int len1 = 3; // dynamnic value calculated by another func
    int len2 = 3; // dynamnic value calculated by another func
    
    int rows = len2 + 1;
    int cols = len1 + 1;
    
    int (*twodarr)[cols] = malloc( sizeof(int[rows][cols]) );
    
    for (int r = 0; r < rows; r++) {
        for (int c = 0; c < cols; c++) {
            twodarr[r][c] = my_val;
            printf("%d ", twodarr[r][c]);
        }
        puts("");
    }
    
    free(twodarr);
    
    return 0;
}
Lundin
  • 195,001
  • 40
  • 254
  • 396
  • I like this approach, but when I try it it throws following errors: error C2057: expected constant expression error C2466: cannot allocate an array of constant size 0 error C2087: 'abstract declarator': missing subscript warning C4034: sizeof returns 0 error C2036: 'int (*)[0]': unknown size – Stefan Jun 29 '21 at 10:00
  • 1
    @Stefan You need to use a standard C compiler. You appear to be using some obsolete one from 1989. – Lundin Jun 29 '21 at 10:17
  • 2
    @Stefan: the problem is that Variable Length Arrays are an optional feature, and that Microsoft choosed to not support them. – Serge Ballesta Jun 29 '21 at 10:17
  • 2
    @Lundin: VLA are optional since C11, and MSVC 2019 correctly defines the constant `__STDC_NO_VLA__` to declare that is does not support VLAs. – Serge Ballesta Jun 29 '21 at 10:19
  • 1
    @SergeBallesta Who cares, that's not a C compiler. `#ifndef __STDC__ #error Trash! #endif`. – Lundin Jun 29 '21 at 10:29
  • @Lundin: I tried to install a new compiler and now have: gcc (MinGW.org GCC-6.3.0-1) 6.3.0 The errors are the same .. I hope there is a way how to fix this, cause I really love this approach .. :) – Stefan Jun 29 '21 at 14:05
  • @Stefan No idea what you are doing wrong. Compiles & runs cleanly on gcc 6.3.0, max warnings enabled. https://godbolt.org/z/YEG4an4PK Maybe you are compiling it as C++ or something. – Lundin Jun 29 '21 at 14:26
2

Arrays have never been first class elements in C, and multi-dimensional ones have even more poor support. Originally, only C used constant sized arrays, because pointer arithmetics was enough for dynamic 1D arrays, and pointers were an essential element of the language.

C99 introduced the concept of Variable Length Arrays which are what @Lundin's answer uses. Unfortunately, C11 defined them as an optional feature, and Microsoft choosed not to support them for compatibility with C++.

If you use a Microsoft compiler or want compability with environments that do not support the optional VLA feature, you will have to use the old linear idiom: you only use 1D arrays and use compound indices computation: the index of element (i, j) is j + i * cols where cols is the size of the second dimension.

Your code could become:

...
int *twodarr = malloc(rows * cols * sizeof(int)); // BEWARE: actualy 1D array!

for (x = 1; x < cols; x++) {
    for (y = 1; y < rows; y++) {
        twodarr[x + y*cols] = my_val;
    }
}
...
Serge Ballesta
  • 143,923
  • 11
  • 122
  • 252
  • 2
    One problem with this is old "mangled array" style is that it's so bug prone. `x + y*cols` should have been `x*rows + y` yeah? I've written similar bugs too a fair amount of times. – Lundin Jun 29 '21 at 10:20
  • 1
    @Lundin I do not want to discuss whether abandonning VLAs in C11 was clever or stupid... But the fact is that conformant environment can choose to not support them. – Serge Ballesta Jun 29 '21 at 10:22
  • Regardless, you appear to have a bug in your index calculation here. – Lundin Jun 29 '21 at 10:30
  • 1
    I really prefer Lundin's solution but do to tech limitations I have to go with this ... It's not bad, if one doesn't do mistakes :D Thank you all, for the great response. It helped a lot :) – Stefan Jun 29 '21 at 17:51
1

@kcabus had it right...and admittedly the much more readable way for sanity sake.

The other way to go about it would be to declare it as a memory block, but its much more confusing.

such as

int *twodarr = (int*)calloc((rows * 
cols), sizeof(int));
// accessed as follows
*(twodarr + rows*r + c) = value;
// rows * position + position 2
// much more confusing.

A third alternative would be to create a struct like POINT (or just use point) and use two values by just creating an array of POINT just as an example. But I assume you don't want to deal with that in a loop...and I don't blame you heh.

BrainOverflow
  • 34
  • 1
  • 1
  • 7
  • 1
    I thought you were using calloc for some odd reason, to use malloc you would just multiply instead of the two arguments if you need to do it that way. int *twodarr = (int*)malloc(rows * cols * sizeof(int)); (it won't let me use a code block in a comment from my phone...thats wrong) it removed the pointer. – BrainOverflow Jun 29 '21 at 11:28