segmentation fault on strcpy();

Question

#include <stdio.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <netdb.h> 
#include <string.h>
#include <strings.h>
#include <stdlib.h>
#include <unistd.h>
#include <arpa/inet.h>
#include <pthread.h>


typedef struct client
{
    int threadid;
    int argc;
    char *argv[3];
} client;

void exit(int status);
void error(char *msg);

void *threadClient(void *socket_desc);

int main(int argc, char *argv[])
{
    client info[10];
    pthread_t thread[10];

    printf("%s\n%s\n%s\n", argv[0], argv[1], argv[2]);

    // Error happens here

    for (int i=0; i<=10; i++)
    {
        info[i].threadid = i;
        strcpy(info[i].argv[0], argv[0]);
        strcpy(info[i].argv[1], argv[1]);
        strcpy(info[i].argv[2], argv[2]);
        info[i].argc = argc;
        printf("here");
        if (pthread_create(&thread[i], NULL, threadClient, (void*)&info[i]) < 0)
        {
            perror("could not create thread");
            return 1;
        }
        sleep(3);
    }
    pthread_exit(NULL);
    return 0;

}

During the loop, when I am trying to copy the info from argv to my struct I get a segmentation fault. Why does it happen?

Program received signal SIGSEGV, Segmentation fault. __strcpy_sse2_unaligned () at ../sysdeps/x86_64/multiarch/strcpy-sse2-unaligned.S:296 296 ../sysdeps/x86_64/multiarch/strcpy-sse2-unaligned.S: No such file or directory.

1) `i<=10` --> `i<10` 2) `strcpy(info[i].argv[0], argv[0]);` `info[i].argv[0]` doesn't initialized. — BLUEPIXY, Apr 20 '17 at 19:26
i changed the for loop i problem thank you i didn't noticed i put that there. but how would i go about initializing it? — jhowe, Apr 20 '17 at 19:41
`info[i].argv[0] = malloc(strlen(argv[0])+1);` before `strcpy`. — BLUEPIXY, Apr 20 '17 at 20:02

score 1 · Accepted Answer · answered Apr 20 '17 at 20:23

There are two problem here.

First, your argv array in your info structure is an array of pointers. These start out uninitialized. When you later call strcpy, giving one of these array elements as the first parameter, it expects that pointer to point to valid memory. So you end up dereferencing an uninitialized pointer. This invokes undefined behavior, which in this case manifests as a segfault.

You need to assign something to these pointers. You can either use strdup to make a copy of these strings:

info[i].argv[0] = strdup(argv[0]);
info[i].argv[1] = strdup(argv[1]);
info[i].argv[2] = strdup(argv[2]);

Or, if you don't plan on modifying these values, you can just copy the pointer values directly:

info[i].argv[0] = argv[0];
info[i].argv[1] = argv[1];
info[i].argv[2] = argv[2];

The second issue is an off-by-one error in your loop:

for (int i=0; i<=10; i++){

Because you use <=, your indexes into the array will range from 0 to 10. However, your array only has 10 elements (with indexes 0 to 9), so you're writing past the end of the array. This also invokes undefined behavior.

Change your conditional to < as follows:

for (int i=0; i<10; i++){

score 0 · Answer 2 · answered Apr 20 '17 at 20:15

Your struct is defined to contain three pointers (char *argv[3]). When you create an array of those structs in the stack (client info[10]) space is reserved for these pointers, among other things. The structs aren't initalized to anything, so the pointers don't point to any sensible memory location. Hence, accessing them as with strcpy(info[i].argv[0], argv[0]); is in error.

Either allocate the space you use explicitly:

info[i].argv[0] = malloc(strlen(argv[0])+1);
strcpy(info[i].argv[0], argv[0]);

Or use strdup() to copy the strings:

info[i].argv[0] = strdup(argv[0]);

Remember to free() the space you allocated afterwards.

Also, I don't think you should declare a function called exit() yourself, it's a standard function and should be in stdlib.h.

segmentation fault on strcpy();

2 Answers2