0

I have an issue with pointers to struct that have members that are pointers to struct too.

Browsing the suggested similar questions I found out this:

Accessing elements within a pointer of a struct inside another pointer to a struct

where people suggest to pay attention to allocation memory of the structure.

I think this is done correctly in my code.

typedef struct {
  int id_vec;
  float *vec_value;
} Vector;

typedef struct cluster{
  int id_cluster;
  float *centroid;
  Vector *patternInCluster; 
} Cluster;

int main(void){
  Cluster *cluster_ptr= malloc(3 * sizeof(Cluster));
  if (cluster_ptr==NULL){
    printf("NULL");
  }
  cluster_ptr->patternInCluster=malloc(2 * sizeof(Vector *));
  if (cluster_ptr->patternInCluster==NULL){
    printf("NULL");
    cluster_ptr->patternInCluster=NULL;
  }

  float p1[3]={0.0f,1.0f,2.0f};
  Vector *somePattern=malloc(2 * sizeof(Vector));
  somePattern[0].id_vec=1;
  somePattern[0].vec_value=p1;
  somePattern[1].id_vec=2;
  somePattern[1].vec_value=p1;
}

Then I expect that this statement works:

cluster_ptr[1].patternInCluster[1]=somePattern[1];

But it compiles and produces a Segmentation Fault.

Unexpectedly, the following statement doesn't get errors:

cluster_ptr[0].patternInCluster[1]=somePattern[1];

and a test show me correct result(somePattern[1] id and value)

I tried to debug with gdb but I only can see this:

Program received signal SIGSEGV, Segmentation fault. 0x00005555555547fe in main () at test_struct.c:36 36 cluster_ptr[1].patternInCluster[1]=somePattern[1];

Am I missing some allocation mistakes?

Improve this question
2
  • OT: regarding: typedef struct { int id_vec; float *vec_value; } Vector; 1) always include a 'tag' name because most debuggers use the 'tag' name to access the individual fields of the struct. 2) for flexibility, separate the definition of the struct from a typedef for that struct Commented Jan 10, 2019 at 19:53
  • The posted code does not compile! So how you expect us to be able to reproduce the problem and then help you to debug it? Commented Jan 10, 2019 at 19:54

3 Answers 3

Reset to default
1

It's because you're not populating things fully.

This line

cluster_ptr->patternInCluster=malloc(2 * sizeof(Vector *));

is the same as saying

cluster_ptr[0].patternInCluster=malloc(2 * sizeof(Vector *));

and really given that cluster_ptr has been allocated as 3 Cluster it would be more clearer in your code to do the latter.

Since cluster_ptr[1].patternInCluster hasn't been given a value, trying to dereference it will lead to undefined behaviour but more likely will result in a segmentation fault.

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

1

You do not allocate enough memory:

cluster_ptr->patternInCluster=malloc(2 * sizeof(Vector *));

With patternInCluster being of type Vector *, you should allocate memory to hold elements of type Vector, not Vector*.

cluster_ptr->patternInCluster=malloc(2 * sizeof(Vector));
Improve this answer

2 Comments

I think this is true, but doesn't solve the problem.
There are too many problems to solve them all at once. Given that your struct is larger than 1 pointer you access memory that you don't own. It's always hard to get all issues if there are multiple in a question.
0

Your problem is NOT accessing the pointer inside the struct. Your problem is how you are using malloc().

When you have one pointer, you malloc only once:

int *pointer = (int* )malloc(sizeof(int));
*pointer = 1;
printf("*pointer:%d\n", *pointer);

When you have pointer-to-pointer, you malloc() once for the **pointer_to_pointer, but you also have to malloc() once for the *pointer_to_pointer:

int** pointer_to_pointer = (int** )malloc(sizeof(int*));
*pointer_to_pointer = (int* )malloc(sizeof(int));
**pointer_to_pointer = 2;
printf("**pointer:%d\n", **pointer_to_pointer);

And if you have more than one pointer, at the location pointed to by **pointer_to_pointer, you need a for loop to assign memory to every one of those *pointer_to_pointers.

for (unsigned int i = 0; i < 3; i++)
{
    *(pointer_to_pointer + i*sizeof(int)) = (int* )malloc(sizeof(int));
}
**(pointer_to_pointer + sizeof(int)) = 3;
**(pointer_to_pointer + 2UL*sizeof(int)) = 4;
printf("**(pointer_to_pointer + sizeof(int):%d\n", **(pointer_to_pointer + sizeof(int)));
printf("**(pointer_to_pointer + 2UL*sizeof(int):%d\n", **(pointer_to_pointer + 2UL*sizeof(int)));

You are mistaken to think that Cluster *cluster_ptr= malloc(3 * sizeof(Cluster)); will automatically/magically assign memory for Cluster[0] and Cluster[1] and Cluster[2].

Your statement actually assigns memory only for Cluster[0], but big enough for 3 Clusters.

So the the modified code will look like this:

#include <string.h>
#include <stdio.h>
#include <malloc.h>

typedef struct {
    int id_vec;
    float *vec_value;
} Vector;

typedef struct cluster{
    int id_cluster;
    float *centroid;
    Vector **patternInCluster;
} Cluster;

int main(void){

    Cluster **cluster_ptr = (Cluster **)malloc(sizeof(Cluster*));
    for (long unsigned int i = 0; i < 3; i++) {
        cluster_ptr[i] = (Cluster *)malloc(sizeof(Cluster));
        if (cluster_ptr[i]==NULL){
            printf("NULL");
        }

        cluster_ptr[i]->patternInCluster = (Vector **) malloc(sizeof(Vector*));
        for (long unsigned int j = 0; j < 3; j++) {
            (*cluster_ptr)->patternInCluster[j] = (Vector *) malloc(sizeof(Vector));
            if ((*cluster_ptr)->patternInCluster[j]==NULL){
                printf("NULL");
                (*cluster_ptr)->patternInCluster[j]=NULL;
            }
        }
    }



    float p1[3]={0.0f,1.0f,2.0f};
    Vector *somePattern= (Vector *) malloc(sizeof(Vector));
    somePattern[0].id_vec=1;
    somePattern[0].vec_value=p1;
    somePattern[1].id_vec=2;
    somePattern[1].vec_value=p1;

    cluster_ptr[1]->patternInCluster[1] = &somePattern[0];
    cluster_ptr[0]->patternInCluster[1] = &somePattern[1];
    cluster_ptr[1]->patternInCluster[0] = &somePattern[1];
    cluster_ptr[2]->patternInCluster[1] = &somePattern[0];

    printf("%d\n", cluster_ptr[1]->patternInCluster[1]->id_vec);
    printf("%d\n", cluster_ptr[0]->patternInCluster[1]->id_vec);
    printf("%d\n", cluster_ptr[1]->patternInCluster[0]->id_vec);
    printf("%d\n", cluster_ptr[2]->patternInCluster[1]->id_vec);

    return 0;
}
  • On my system, I just compiled and it builds and runs error-free.
Improve this answer

3 Comments

Thank you very much for the nice explanation. In the code, you declare cluster_ptr has **Cluster but this sounds weird to me, because I want it to point to a "vector" of struct. As you said, I misunderstood that Cluster *cluster_ptr= malloc(3 * sizeof(Cluster)) allocate enough memory for 3 pointer to struct, while it is only giving that space to cluster_ptr[0]. Then why change the type of Cluster *` to Cluster ** when I can just allocate memory for each *cluster_ptr[i].patternInCluster in a loop?
@LucaJungla, you can try removing the ** from the Cluster pointer and then replace all . with -> and then run the program and see if it produces the right output. (Hint: It does not.)
it is not what I mean for(i=0;i<k;i++) { cluster_ptr[i].patternInCluster=malloc(2 * sizeof(Vector *))} then all this assignment :cluster_ptr[0].patternInCluster[0]=somePattern[0]; cluster_ptr[1].patternInCluster[0]=somePattern[1]; etc... works and print the right value

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.