CODEWITHSUNDEEP
cmemorycrashmalloc
Bar
Bar

Reputation: 194

Code crashes on c, most likely something to do with malloc?

this is quite basic but I've been trying to solve it for a few hours now with no success. This code is supposed to read 3 ints from user input and then, points according to the ints (n = how many, d = dimension, k is unrelated to this part) and create an array of them. For some reason it crashes on the second loop, where it fails to create the second point, but im not sure why. I think it might has something to do with malloc but I'm totally lost, would appreciate any help.

I inserted the input :

5 3 2 
1.2 3.4 0.1

2 times before it crashed.

Code is below:

int main(){

    double* pdata;
    int n,d,k;
    scanf("%d %d %d",&n,&d,&k );
    SPPoint*  parr [n];
    for(int i=0; i<n; i++)
    {
        double darr [d];
        for(int j = 0; j < d-1; j++)
        {
            scanf(" %lf", &darr[j]);
        }
        scanf(" %lf", &darr[d-1]);
    pdata = darr;
    parr[i] = spPointCreate(pdata, d, i); 
    }
}

This is the code for the spPointCreate function:

struct sp_point_t{
double* data;
int dim;
int index;
};

SPPoint* spPointCreate(double* data, int dim, int index){
SPPoint* point = malloc(sizeof(SPPoint*));
if(point == NULL)
{
    return NULL;
}
point->data = (double*) malloc(sizeof(data));
for( int i=0 ; i<dim ; i++)
{
    point->data[i] = data[i];
}
point->dim = dim;
point->index = index;
return point;
}

Upvotes: 3

Views: 285

Answers (2)

chux
chux

Reputation: 153338

Code is mis-allocating in 2 places

// Bad
SPPoint* spPointCreate(double* data, int dim, int index){
  SPPoint* point = malloc(sizeof(SPPoint*));  // allocate the size of a pointer
  ...
  point->data = (double*) malloc(sizeof(data)); // allocate the size of a pointer

Instead avoid mis-coding the type and allocate to the the size of the de-referenced variable.

Also need to allocate N objects.

SPPoint* spPointCreate(double* data, int dim, int index){
  size_t N = 1;
  SPPoint* point = malloc(sizeof *point * N);// allocate the size of `*point` * N

  ...
  assert(dim >= 0);
  N = dim;
  point->data = malloc(sizeof *(point->data) * N);

BTW, casting the result of malloc() not needed.

2nd allocation would benefit with a NULL check. More complicated as dim may be 0 and a malloc() return of NULL in that case is OK.

  N = dim;
  point->data = malloc(sizeof *(point->data) * N);
  if (point->data == NULL && N > 0) {
    free(point);
    return NULL;
  }

Upvotes: 1

A.S.H
A.S.H

Reputation: 29332

SPPoint* point = malloc(sizeof(SPPoint*));

should be: struct SPPoint* point = malloc(sizeof(*point));

point->data = (double*) malloc(sizeof(data));

should be point->data = malloc(dim * sizeof(*point->data)); since you want to allocate dim doubles for your point.

Upvotes: 3

Related Questions