1

I have been trying to execute the following code. Though the task is trivial, I am getting a segmentation fault.

The aim of the code snippet betlow is to create a multi dimensional array of maximum row size 4 and column size 33. Then after creation, it should set the contents of all the rows as 0, followed by a '\0' character. Then in the end, it should display the output on the stdout.

Although I am not new to programming, I keep on getting similar errors, so if possible please explain me how can I avoid such mistakes in the future.

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

int main(int argc, char* argv[])
{
        int i,j,k,x,y;
        char** arr;
        arr = (char**) malloc(4 * sizeof(char*));
        for ( i = 0; i < 4; i++) {
                arr[i] = (char*) malloc(9 * sizeof(char));
                memset(arr,0,8);
                arr[i][8] = '\0';
        }
        for ( j = 0; j<4; j++) {
                puts(arr[j]);
        }
        return 0;
}
Cody Gray - on strike
  • 239,200
  • 50
  • 490
  • 574
uyetch
  • 2,150
  • 3
  • 28
  • 33
  • 4
    Casting `malloc` is generally wrong, as it can potentially hide errors. ([Relevant reading](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc)) – Cody Gray - on strike Jan 10 '12 at 00:27
  • The thing you are building here is often called a "ragged array" or "jagged array" and it is different from a vanilla two dimensional array such as you can declare with `char arr[33][4]`; Ragged arrays are only one of several solutions to the problem of dynamic multidimensional arrays in c. – dmckee --- ex-moderator kitten Jan 10 '12 at 00:29
  • You also need to free() the memory blocks allocated with malloc(). Add for(j=0;j<4;++j)free(arr[j]);free(arr); before return 0; – al01 Jan 10 '12 at 00:34

3 Answers3

5

You're memseting the wrong pointer.

Instead of:

memset(arr,0,8);

you want:

memset(arr[i],0,8);

So you're off by one level of indirection.


As pointed out in the comments, here are some optimizations:

for ( i = 0; i < 4; i++) {
    arr[i] = (char*) malloc(9 * sizeof(char));
    memset(arr[i],0,9);
}

or

for ( i = 0; i < 4; i++) {
    arr[i] = (char*) calloc(9, sizeof(char));
}

Note that the cast to char* isn't necessary in C.


If you wanted to the character '0' instead of the null-character, then you should go with this:

for ( i = 0; i < 4; i++) {
    arr[i] = (char*) malloc(8 * sizeof(char));
    memset(arr[i],'0',8);
    arr[i][8] = '\0';
}
Mysticial
  • 464,885
  • 45
  • 335
  • 332
  • 1
    +1: and the code should be using `memset(arr[i], '\0', 9);` so that the separate assignment of the extra 0 byte is not needed. – Jonathan Leffler Jan 10 '12 at 00:29
  • Or just use `calloc()` instead of `malloc()` and forget about it :) – Brian Roach Jan 10 '12 at 00:29
  • 1
    +1 for addressing the seg-fault; but from the description, I think the OP *might* actually need `memset(arr[i],'0',8)` (setting each element to a length-eight null-terminated string of zeroes). – ruakh Jan 10 '12 at 00:30
  • @ruakh It could be, it's not entirely clear if the OP wants '\0' or '0'. – Mysticial Jan 10 '12 at 00:31
  • The OP wants to fill the string with '0', printing '\0' would not produce any output. – al01 Jan 10 '12 at 00:39
1
memset(arr,0,8);

is the issue, you are clearing the array of pointers

try:

memset(arr[i], '0', 8);

assuming you want to fill the string with zeros

al01
  • 122
  • 1
  • 4
0

in the for loop I think you have an logical error:

memset(arr[i],0,8);

you also don't need to set this:

arr[i][8] = '\0';

instead you could do this:

memset(arr[i],0,9);

the '\0' character is nothing different then 0.

v01pe
  • 1,096
  • 2
  • 11
  • 19