0

I'm trying to use a function to create a 2d character array, set all characters to character x, and print the first 10 rows to test if it was done correctly.

My code compiles using: gcc -std=c89 -pedantic code.c However, when I try to run the code using ./a.out, I get a segmentation fault.

I have isolated the problem to when I try to print my array, as there is no segmentation fault when the line printf("%c", a[i][j]); is commented out.

#define rows 100
#define columns 3

char** makeArr(int rows, int columns)
{
    int temp,i,j;
    char** a = (char**)malloc(rows*sizeof(char*));
    for(temp = 0; temp < rows; temp++)
    {
        a[temp]=(char*)malloc(columns*sizeof(char));
    }

    memset(a, 'x', rows*columns*sizeof(a[0][0]));

    for(i = 0; i < 10; i++)
    {
        for(j = 0; j < columns; j++)
        {
            printf("%c", a[i][j]);
        }
        printf("\n");
    }

    return a;
}

Any help with this problem would be greatly appreciated, and if any more context is needed, just let me know.

sdfrn42
  • 15
  • 7
  • 8
    `memset` fills the continuous region of memory. Since you allocate each 1D-array separately, they are not guaranteed to form the continuous region. You have to memset all 1D-arrays separately. –  Jul 02 '19 at 21:28
  • 3
    Your array is not a 2D array but a 1D array of pointers to 1D arrays. This is not the same. – DYZ Jul 02 '19 at 21:30
  • 2
    [don't cast malloc](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) – Barmar Jul 02 '19 at 21:32
  • I tried removing the casts from malloc as suggested by Barmar but I am still getting a segmentation fault – sdfrn42 Jul 02 '19 at 21:42
  • 1
    This is relevant: [**Correctly allocating multi-dimensional arrays**](https://stackoverflow.com/questions/42094465/correctly-allocating-multi-dimensional-arrays) – Andrew Henle Jul 02 '19 at 21:47

2 Answers2

2

You can allocate the array as a contiguous block:

char (*arr)[COLUMNS] = malloc(sizeof *arr * ROWS);
memset(arr, 'x', sizeof *arr * ROWS);

The memset call is going beyond the bounds of the allocated array a:

memset(a, 'x', rows*columns*sizeof(a[0][0]));

The array is comprised of char* pointers and has rows elements. Since you're allocating the sub-arrays individually you'd need separate memset calls for each malloc'd pointer. Assuming the strings have a fixed length, it would be simpler to allocate the storage as one continuous block.

jspcal
  • 50,847
  • 7
  • 72
  • 76
2

The easiest solution is to do the memset in the same loop you malloc the rows in:

for(temp = 0; temp < rows; temp++) {
    a[temp] = malloc(columns*sizeof(char));
    memset(a[temp], 'x', columns);
}

Notice that I do not multiply columns * sizeof(a[0][0]). sizeof(a[0][0]) is the size of a char in bytes, so usually 1. If you were to do columns * sizeof(a[0][0]) it would still work on most any system, but in practical use, you really do not need it since sizeof(char) is almost universally 1.

anonmess
  • 465
  • 3
  • 9