CODEWITHSUNDEEP
csegmentation-faultmallocstrcpy
Vitalij Kornijenko
Vitalij Kornijenko

Reputation: 559

C - segmentation core dump on strcpy()

I get a segmentation fault each time I'm trying to run this function.

char *hist_array[20];
int history_counter = 0;

void save_to_history(char *temp){
    temp = malloc(512);/*512 is the size of temp array*/
    printf("temp = %s\narray = %s",temp,hist_array[history_counter]);/*debug*/
    strcpy(hist_array[history_counter],temp);
    printf("Saved %s to history to %d\n\n",hist_array[history_counter],history_counter);
    history_counter++;
}

I'm not sure whether I'm using malloc correctly or not, but as I understand it should help with properly saving my string temp to an array of strings hist_array. Also, temp is never NULL.

EDIT 1: Changed sizeof(temp) to its proper size 512, still getting segfault.

Upvotes: 0

Views: 2712

Answers (2)

Devolus
Devolus

Reputation: 22084

In your code, you are allocating via malloc but you don't initialize it. So when you do this:

printf("temp = %s\narray = %s",temp,hist_array[history_counter]);/*debug*/
strcpy(hist_array[history_counter],temp);

The should print some garbage and the subsequent strcpy will copy whatever is in temp at the time, until it encounters a 0 byte. You are invoking undefined behaviour here, because you should initialize the memory.

For example you could do:

memset(temp, 0, 512);

Additionally you are dereferencing pointers in your array hist_array. Since this is a global variable, all the pointers will originally be NULL pointers, which is causing your segfaul there.

Upvotes: 2

ajay
ajay

Reputation: 9680

The problem is with the following statement - 

strcpy(hist_array[history_counter], temp);

strcpy tries to copy the buffer pointed to by temp to the buffer pointed to by hist_array[history_counter] but char *hist_array[20]; defines hist_array to be an array of 20 pointers to characters. You should change your function to - 

char *hist_array[20];
int history_counter = 0;

void save_to_history(char *temp) {
    // +1 for the terminating null byte as strlen
    // does not count the null byte

    hist_array[history_counter] = malloc(1 + strlen(temp)); 
    if(hist_array[history_counter] == NULL) {
        printf("not enough memory to allocate\n");
        // handle it
    }
    strcpy(hist_array[history_counter], temp);
    printf("Saved %s to history to %d\n\n",
           hist_array[history_counter], 
           history_counter);

    history_counter++;
}

Upvotes: 3

Related Questions