Tek-Tips is the largest IT community on the Internet today!

Members share and learn making Tek-Tips Forums the best source of peer-reviewed technical information on the Internet!

  • Congratulations SkipVought on being selected by the Tek-Tips community for having the most helpful posts in the forums last week. Way to Go!

Segfault when indexing an array from function!

Status
Not open for further replies.

taskler

Programmer
Sep 7, 2010
5
0
0
SE
I am getting a segfault when running this, somehow because I'm using double* d wrong in vector_make_from.
I need to use -> and pointers somehow I guess but don't get how.
Should vector_make and *from return a pointer to a vector nit a vector?




______________________________



#include <math.h>
#include <stdlib.h>
#include "vector.h"
#include <stdio.h>

vector vector_make(int size)
{
vector vr;// = malloc(sizeof(vector));
vr.size = size;
vr.v = malloc(size * sizeof(double));
return vr;
}

vector vector_make_from(int vsize, double* d)
{
vector vr;// = malloc(sizeof(vector));
vr.size = vsize;
int i;
for(i=0; i<vsize; i++) {
vr.v = d;
}
return vr;
}





typedef struct
{
int size;
double* v;
} vector;

vector vector_make(int size);
vector vector_make_from(int size, double* d);









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


int main(void)
{

vector a = vector_make(3);
vector b = vector_make(3);

double z[3] = {5,1,3};

vector c = vector_make_from(3,z);
printf("\nWhere is jsjsjerror?");
vector_set(c,2,4);
printf("\nz[2] is:%f\n", z[2]);

double x[3] = {1,2,3};
double y[3] = {2,3,5};
a.v = x;
b.v = y;



return 0;
}


 
One advice:

The code below almost always is erroneous ...

Code:
vector vector_make(int size)
{
  vector vr;// = malloc(sizeof(vector));
  vr.size = size;
  vr.v = malloc(size * sizeof(double));
  return vr;
}

That's because you use a local variable vr (whose lifespan doesn't go beyond the end of the function and return it as the function return value.

So the caller will end up with the vr which after the end of the vector_make function does not have a valid memory area anymore. It only has a valid memory area while execution is inside the vector_make function body. So at any point, someone could call another function and the local variables of that function could end up in the same memory area as the vr object. That means that the client of vector_make will think it has exclusive access to that area through a vector object, but another function will allocate its locals there too, and the two will never know of each other, but will write the same memory area, messing around with each other's business. The main problem is that even if there is a bug, if you use stupid compilers you'll never know and the bug may even fail to manifest, or it could manifest straight away. You'll never know. A bug that doesn't manifest or manifests randomly is worse than any other bug because you may have problems reproducing the bug on the next run, to correct the problem.

The correct value of the code would be (using pointers, of course ... and I'll explain why it solves the problem):

Code:
vector* vector_make(int size)
{
  vector* vr = (vector*) malloc(sizeof(vector));
  vr->size = size;
  vr->v = (double*) malloc(size * sizeof(double));
  return vr;
}

So you declare the function such as it returns a pointer to a vector. You do the creation almost the same inside the function and return the address of the vector you just created, not the vector itself.

Now there is no problem in returning basic data types from a function (integers, doubles). There also isn't any problem with returning pointers, because typically they are just addresses and addresses are like integers. But there is a problem if you return a local struct from a function.

The last piece of code allocates the vector object from within the function body. This allocation alone solves your previous problem 1. because it allocates in the heap (which is in a different memory area than the stack) and 2. because the allocation process guarantees that nobody will be allocated in the same area until you manually free that vector object by using free. So control is in your hands now, rather than in the hands of destiny (like in the first code fragment)
 
vector_make is Ok because it returns a structure VALUE, not a reference to the local vr.

You forgot to initialize member v pointer (in other words, you don't allocate memory for vector body) in vector_make_from: that's why you catch segment fault error. The function returns vector structure with undefined v pointer value.

And where is vector_set code?
 
Status
Not open for further replies.

Part and Inventory Search

Sponsor

Back
Top