CODEWITHSUNDEEP
cmultithreadingsegmentation-fault
Sonu Mishra
Sonu Mishra

Reputation: 1779

Unable to pin down a bug in a simple multithreading program

I am working on a project that involves multi-threading. Although I have a decent understanding of multi-threading, I have not written many such codes. The following code is just a simple code that I wrote for hands-on. It works fine when compiled with gcc -pthread.

To build upon this code, I need to include some libraries that already have pthread included and linked. If I compile by including and linking those libraries, 3 out of 5 times it gives segmentation fault. There is some problem with the first for-loop in main() -- replacing this for-loop with multiple statements does the work.

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

#define NUM_THREADS 3

pthread_mutex_t m_lock = PTHREAD_MUTEX_INITIALIZER;

typedef struct{
    int id;
    char ip[20];
} thread_data;

void *doOperation(void* ctx)
{
    pthread_mutex_lock(&m_lock);
    thread_data *m_ctx = (thread_data *)ctx;
    printf("Reached here\n");
    pthread_mutex_unlock(&m_lock);
    pthread_exit(NULL);
}

int main()
{
    thread_data ctx[NUM_THREADS];
    pthread_t threads[NUM_THREADS];

    for (int i = 0; i < NUM_THREADS; ++i)
    {
        char ip_n[] = "127.0.0.";
        char ip_h[4];
        sprintf(ip_h, "%d", i+1);
        strcpy(ctx[i].ip, strcat(ip_n, ip_h));
    }

    for (int i = 0; i < NUM_THREADS; ++i)
    {
        pthread_create(&threads[i], NULL, doOperation, (void *)&ctx[i]) 
    }

    for (int i = 0; i < NUM_THREADS; ++i)
    {
        pthread_join(threads[i], NULL);
    }

    pthread_exit(NULL);

}

Upvotes: 1

Views: 50

Answers (4)

NayabSD
NayabSD

Reputation: 1120

I just added ; at the end of pthread_create(&threads[i], NULL, doOperation, (void *)&ctx[i])

And this segmentation fault might be because of

char ip_n[] = "127.0.0.";

Above, the sizeof(ip_n) only returns 9. But you need atleast 10 characters to store string like 127.0.0.3 (Including null character at the end). Unauthorized memory access might result in segmentation fault. Try replacing this with char ip_n[10] = "127.0.0.";

Upvotes: 1

clarasoft-it
clarasoft-it

Reputation: 176

ip_n[] points to a constant; the compiler reserved 9 bytes including the NULL byte. Anything after these 9 bytes should not be accessed and if you do, the result is undefined (it could work some of the time but likely not all the time). When you do:

strcat(ip_n, ip_h)

You are overflowing the buffer pointed to by ip_n. Maybe this is what is causing your problem. If it's not, I still recommend fixing this.

Upvotes: 0

John Bollinger
John Bollinger

Reputation: 181244

You say the code you present "works fine", but it is buggy. In particular, the first for loop is buggy, so it is not surprising that it gives you trouble under some circumstances. Here's a breakdown:

    char ip_n[] = "127.0.0.";

You have declared ip_n as an array of char exactly long enough to hold the given initializer, including its terminating null character.

    char ip_h[4];
    sprintf(ip_h, "%d", i+1);

Supposing that the sprintf() succeeds, you have written a non-empty string into char array ip_h.

    strcpy(ctx[i].ip, strcat(ip_n, ip_h));

You attempt via strcat() to append the contents of ip_h to the end of ip_n, but there is no room -- this writes past the bounds of ip_n, producing undefined behavior.

The easiest way to solve this would probably be to declare ip_n with an explicit length that is sufficient for the full data. In general, a dotted-quad IP address string might require as many as 16 bytes, including the terminator:

    char ip_n[16] = "127.0.0.";

Upvotes: 3

user3386109
user3386109

Reputation: 34839

You can't strcat(ip_n, ip_h) because the array ip_n is only big enough to hold the string "127.0.0.". Here's what the man page says, emphasis added

The strcat() and strncat() functions append a copy of the null-terminated string s2 to the end of the null-terminated string s1, then add a terminating `\0'. The string s1 must have sufficient space to hold the result.

The declaration should be

char ip_n[20] = "127.0.0.";

Upvotes: 1

Related Questions