0

This piece of code causes segmentation fault:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define SLIDINGWINDOW 5
#define SEGMENTSIZE 100

int main() {
    char** send_buffer = (char**) malloc (SLIDINGWINDOW);
    int i;
    for (i = 0; i<SLIDINGWINDOW; ++i) {
        send_buffer[i] = (char*) malloc (SEGMENTSIZE);

    }
    for (i = 0; i<SLIDINGWINDOW; ++i) {
        strcpy(send_buffer[i], "Hello, world");
        printf("%s\n", send_buffer[i]);
        fflush(NULL);
    }
}

The strange thing is if you put the content of the second loop into the first loop, it works!

Could anyone see why this happens? Many thanks!

Max
  • 3,824
  • 8
  • 41
  • 62

2 Answers2

6

The size passed to malloc() isn't correct. You probably meant:

char** send_buffer = malloc (SLIDINGWINDOW * sizeof(send_buffer[0]));

and

send_buffer[i] = malloc (SEGMENTSIZE * sizeof(send_buffer[i][0]));

This is because the argument to malloc() is the number of bytes you're asking for, so you need to multiply the length you want by the size of the elements.

In addition, you should also check that the value returned from malloc() isn't NULL.

Note that I've removed the cast from the malloc call - in C, the cast isn't required, and can sometimes mask an error.


A further improvement would be to use strncpy() to ensure that you're not going to write characters after the end of the segment:

strncpy(send_buffer[i], "Hello, world", SEGMENTSIZE);

Note that in the case that the source string is larger than SEGMENTSIZE, then the destination string will not be null terminated. You can fix this with a:

send_buffer[i][SEGMENTSIZE - 1] = '\0';

after the strncpy().

Community
  • 1
  • 1
Timothy Jones
  • 21,495
  • 6
  • 60
  • 90
2

This will produce only enough space for SLIDINGWINDOW bytes:

malloc (SLIDINGWINDOW)

You want enough space for SLIDINGWINDOW pointers:

malloc (SLIDINGWINDOW * sizeof(char*))

Your second malloc() is correct by luck; sizeof(char) is always 1.

chrisaycock
  • 36,470
  • 14
  • 88
  • 125
  • 2
    A more robust way to write it is `char **send_buffer = malloc (SLIDINGWINDOW * sizeof *send_buffer);`; more generally: `some_type *foo = malloc(COUNT * sizeof *foo);` – Keith Thompson Aug 27 '12 at 04:14