2
votes

I have this code but it has an error: Segmentation Fault(core dumped) and it doesn't work with more the 2 threads. Any idea of what am i doing wrong?

This code is for calculate pi by Leibniz formula

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


#define NUM_HILOS 2

struct datos
{
    int inicio;
    int fin;
    float *pi;
}

*calcPi (void *datos){
    struct datos *datos_proceso;
    datos_proceso = (struct datos *) datos;
    int i = datos_proceso -> inicio;
    int end = datos_proceso -> fin;
    printf("inicio  %d \n", i);
    printf("fin     %d \n", end);   
    float *pi = datos_proceso -> pi;
    int signo = 1;
    do{ 
        *pi = *pi +(signo*4.0)/((2*i)+1);
        i++;
        signo *= -1;
        //printf("%f \n", *pi);
    }while(i<end);
}


int main()
{
    int error, i;
    float *pi;
    int j = -1;
    /*variable para hilos*/

I think that the error is over here but i don't know how to fix it

    struct datos hilo_datos[NUM_HILOS];
    pthread_t idhilo[NUM_HILOS];
    //printf("este es pi   %f \n", *pi);
    for(i=0; i<NUM_HILOS; i++)
        {
        hilo_datos[i].inicio =j+1;
        hilo_datos[i].fin =j+1000;
        hilo_datos[i].pi = pi;
        printf("%d \n", hilo_datos[i].inicio);
        printf("%d \n", hilo_datos[i].fin);
        j += 1000;
        }

    for(i=0; i<NUM_HILOS; i++)
    {
        error=pthread_create(&idhilo[i], NULL, (void *)calcPi, &hilo_datos[i]);


    }

    for(i=0; i<NUM_HILOS; i++)
        {
        pthread_join(idhilo[i], NULL);
        }
    printf("este es pi   %f \n", *pi);  
return 0;
}
1
Have you tried using GDB to step through your code? I'm pretty sure Martin is right - hilo_datos is a pointer pointing to a memory space enough for only one datos struct. So hilo_datos[0] I think should work but you segfault at hilo_datos[1]. Maybe you wanted to malloc (sizeof (hilo_datos) * NUM_HILOS)? - fpes
Should you call malloc like this struct datos* hilo_datos = (struct datos*) malloc(NUM_HILOS * sizeof (struct datos));)? - Fiddling Bits
You should learn how to run a program in GDB (or another debugger). That should point you straight to where you're using hilo_datos[i]. - roeland
hilo_datos[i].pi = pi;. pi is an uninitialised variable. That is, you have not allocated any memory for pi and then you dereference it within the thread code. - kaylum
Even if float *pi and the memory it pointed to were correctly initialized, it would still be undefined behavior due to unsynchronized writes to it in multiple threads. - EOF

1 Answers

2
votes

Your errors were mainly forgetting simple things like variable initialization. When a pointer such as float *pi; is accessed before being initialized it will almost always cause problems. At the very least it should have raised a compiler warning. By the way, turn on all your compiler warnings. GCC options

Here are a few specifics to get a clean build...

1 add return statement to calcPi function

    ...
    return 0;
}

2 terminate struct datos with a ;

struct datos {
...
};
 ^

3 function:

* calcPi (void *datos){...}

should be:

void * calcPi (void *datos){...}

Or better:

void calcPi (struct datos *d){...} //passing a struct pointer 

4 Initialize your variables before using them. for example:

float *pi; //uninitialized
float *pi = NULL;//initialized pointer
pi = malloc(sizeof(float)*NUM_HILOS);//array of pi with NUM_HILOS elements

then, in following assignment statements use pi[i], ...

hilo_datos[i].pi = pi[i];

Or, just create a simple float: (you do not need a pointer in this case)

float pi = 0;//works just fine for what you are doing
             //no further initialization is needed

Other problems include mis-application of threads, creating inappropriate variable types (i.e. for the way you are using it, float *pi; could just be float pi; negating the need for malloc())

A very simple example of using this algorithm to compute pi (without threading) is included below for illustration:

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

#define NUM_HILOS 10   //array elements (no threads)

struct datos //modified for use as array, no threads
{
    float pi;  //PI
};

void calcPi (struct datos *d)
{
    struct datos *datos_proceso = d;
    float pi = datos_proceso[0].pi;
    int signo = 1;
    int i;

    for(i=0;i<NUM_HILOS;i++)
    {
        pi = pi + (signo*4.0)/((2*i)+1);
        signo *= -1;
        d[i].pi = pi; //change values for NUM_HILOS to see how 
                      //values for pi converge here. 
    }
}

int main()
{
    int error, i;
    float pi = 0;
    int j = -1;

    //your comment: I think that the error... 
    //... (The error you were seeing here was
    //caused by an uninitialized float *pi; which has been
    //changed to float pi = 0; in this example)

    struct datos hilo_datos[NUM_HILOS];
    for(i=0; i<NUM_HILOS; i++)
    {
        hilo_datos[i].pi = 0; //initialize all to zero
    }

    calcPi(hilo_datos);//send array of struct, check all elelments when returned

    for(i=0; i<NUM_HILOS; i++)
    {
        printf("este es pi   %f \n", hilo_datos[i].pi);    
    }
    getchar();
return 0;
}