What is wrong with my dynamically allocated array of pointers to structs?

Question

I have the following program that references array elements through a double pointer.

typedef struct {
  int num1;
  int num2;
  int num3;
} DATA_SET;

typedef struct {
  int          structID;                                              
  DATA_SET     *data_set_array;  // Pointer to an array of DATA_SET structs
} MY_STRUCT;

int main () {
   MY_STRUCT   *Struct1;
   DATA_SET    **DataSetArray;   // Array of pointers

   Struct1 = malloc(sizeof(MY_STRUCT));
   Struct1->data_set_array = malloc(sizeof(DATA_SET*))  //Allocate mem for the pointer to array of DATA_SETs

   DataSetArray = malloc(sizeof(DATA_SET*) * 2)  // Allocate mem for an array of 2 DATA_SET pointers

   DataSetArray[0] = malloc(sizeof(DATA_SET))    // Allocate mem for the actual DATA_SET struct
   DataSetArray[0]->num1 = 1;
   DataSetArray[0]->num2 = 2;
   DataSetArray[0]->num3 = 3;

   DataSetArray[1] = malloc(sizeof(DATA_SET))    // Allocate mem for the actual DATA_SET struct
   DataSetArray[1]->num1 = 1;
   DataSetArray[1]->num2 = 2;
   DataSetArray[1]->num3 = 3;

   memcpy(Struct1->data_set_array, *DataSetArray, sizeof(DATA_SET*);  //Copy data set array into Struct1

When I print all the data out in Struct1, i get:

   Struct1->data_set_array[0].num1 = 1
   Struct1->data_set_array[0].num2 = 2
   Struct1->data_set_array[0].num3 = 3
   Struct1->data_set_array[1].num1 = 50       //This should be 1
   Struct1->data_set_array[1].num2 = 50       //This should be 2
   Struct1->data_set_array[1].num3 = 65       //This should be 3

Seems to be misuse/data corruption for the 2nd element in the array.

I know there's probably different ways to do this, but I wanted to get familiar with referencing the array indices via double pointers. Am I allocating memory properly? I have a feeling the memcpy is incorrect.

You allocated for DataSetArray twice the memory size for Struct1->data_set_array, and you copy half of that data. — Makketronix, Oct 06 '16 at 04:37
Why do you want the variable `DataSetArray` and a memcpy? Just use `Struct1->data_set_array` - it will make your code much simpler. — Support Ukraine, Oct 06 '16 at 04:48
Do you mean modify Struct1->data_set_array[0] explicitly? Don't I need to allocate memory for the structs? — user1224478, Oct 06 '16 at 05:30
"*`DATA_SET **DataSetArray; // Array of pointers`*" here `DataSetArray` is a pointer to pointer to `DATA_SET`. No array anywhere. — alk, Oct 06 '16 at 09:28
"*I wanted to get familiar with referencing the array indices via double pointers.*" this is very unclear, as as it stands does not really make sense. Perhaps it's a language issue? In any case please describe in more detail what you are after. — alk, Oct 06 '16 at 09:32

score 0 · Answer 1 · answered Oct 06 '16 at 04:38

Struct1->data_set_array = malloc(sizeof(DATA_SET*)
/*                                              ^ wrong */

DataSetArray = malloc(sizeof(DATA_SET*) * 2)
/*                                   ^ wrong */

You need to do sizeof(DATA_SET) because that is the actual size of a struct, not a pointer to a DATA_SET struct.

You're making the mistake of allocating only the size of a pointer times 2, which in many cases can be smaller than you'd hope for.

memcpy(Struct1->data_set_array, *DataSetArray, sizeof(DATA_SET*);

This is also wrong. You must copy the sizeof(DATA_SET). Keep in mind that sizeof(DATA_SET) is not equal to sizeof(DATA_SET*). A pointer's size in bytes is the same regardless of the type of pointer.

score 0 · Answer 2 · answered Oct 06 '16 at 04:58

0

What is wrong with my dynamically allocated array of pointers to structs?

DataSetArray is an array of pointers to struct and there is not anything wrong with it.

The problem is 2 other things:

You can't use memcpy on an array of pointers to struct. The memory isn't consecutive.

Further, data_set_array is not an array of pointers to struct so you are trying to copy between incompatible types.

answered Oct 06 '16 at 04:58

Support Ukraine

42,271
4
38
63

Ah. Should I change the data_set_array to a double pointer? DATA_SET **data_set_array; // Pointer to an array of DATA_SET – user1224478 Oct 06 '16 at 05:32
@user1224478 - It depends what you really are looking for. Do you want `data_set_array` to be `an array of structs` or do want `data_set_array` to be an `array of pointers to struct`? That is two different things. Your current code mixes those. In order to help you with the code, we first need to know you really want. – Support Ukraine Oct 06 '16 at 06:00

score -1 · Answer 3 · answered Oct 06 '16 at 05:23

-1

I guess you basically want to create a Double Dimension array. So basically you can do the code like this

struct abc {
    int i;
};

struct abc **arr;
arr = (struct abc **) malloc(sizeof(struct abc) * 2);
arr[0] = (struct abc*)malloc(sizeof(struct abc));
arr[0]->i = 100;
arr[1] = (struct abc*)malloc(sizeof(struct abc));
arr[1]->i = 200;

One thing that you need to remember is that the outer array is just a pointer, so it doesn't need to be sizeof(abc) , it could very well be sizeof(int*) / sizeof (void*) etc.

so basically the above code could be written as

struct abc **arr;
arr = (struct abc **) malloc(sizeof(int *) * 2);
arr[0] = (struct abc*)malloc(sizeof(struct abc));
arr[0]->i = 100;
arr[1] = (struct abc*)malloc(sizeof(struct abc));
arr[1]->i = 200;

Hope this helps in underlying memory management you want to do with your code

answered Oct 06 '16 at 05:23

Daksh Gupta

7,554
2
25
36

In any case this `arr = (struct abc **) malloc(sizeof(...` should be `arr = malloc(sizeof(struct abc *) * 2);` or even better/more robust `arr = malloc(2 * sizeof *arr);`. – alk Oct 06 '16 at 09:25
BTW, there is no need to cast void-pointers in C, nor is it recommended in any way, – alk Oct 06 '16 at 09:26
alk "BTW, there is no need to cast void-pointers in C, nor is it recommended in any way" where did you get that recommendations from? Is it in standard ? if not then it doesn't matter as its only for coders and not the compiler. BTW, I am not sure what you're trying to say in here. May be there is a better way but it has nothing to do with the question? – Daksh Gupta Oct 07 '16 at 15:11
The C11 Standard says that converting pointers to and from `void`-pointers is done implicitly. This is different from C++. – alk Oct 07 '16 at 15:16
Also you might like to read here: http://stackoverflow.com/questions/1565496/specifically-whats-dangerous-about-casting-the-result-of-malloc – alk Oct 07 '16 at 15:22

What is wrong with my dynamically allocated array of pointers to structs?

3 Answers3