2

Here is what I am trying to do:
1. Create an array of struct pointers.
2. Fill the array with malloc'd structs.
3. Then replace one element in the array with a new malloc'd struct
4. Have no memory leaks.

I have written a test program below, but I am getting seg faults due to invalid reads and writes on my call to memcpy. What am I doing wrong?

#include <stdlib.h>
#include <string.h>
struct my_struct {
    int a;
    int b;
};
int main(int argc, char *argv[])
{
    struct my_struct **my_arr;
    my_arr = (struct my_struct **) malloc(10 * sizeof(struct my_struct *));
    int i;
    for (i = 0; i < 10; i++) {
        struct my_struct *my_str = (struct my_struct *) malloc(sizeof(struct my_struct *));
        my_arr[i] = my_str;
    }
    free(my_arr[0]);
    memcpy(my_arr[0], my_arr[1], sizeof(struct my_struct *) * 9);
    my_arr[9] = (struct my_struct *) malloc(sizeof(struct my_struct *));
    for (i = 0; i < 10; ++i) {
        free(my_arr[i]);
    }
    free(my_arr);
}
Yu Hao
  • 119,891
  • 44
  • 235
  • 294
Ryan
  • 658
  • 8
  • 22
  • 2
    The usual warning: [In C you should not cast the result of `malloc`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc), or any other function returning `void *`. – Some programmer dude Oct 23 '15 at 05:02
  • 1
    Also, you can't use `memcpy` to copy overlapping memory, for that you need [`memmove`](http://en.cppreference.com/w/c/string/byte/memmove). Not that the `memcpy` call copy overlapping memory now that I think about it, it won't work as you expect it to that copying, and will give you *undefined behavior*. – Some programmer dude Oct 23 '15 at 05:03
  • Then there's the problem with you allocating ten structures in the "array", but you free ***11*** structures. – Some programmer dude Oct 23 '15 at 05:04
  • `struct my_struct *my_str = (struct my_struct *) malloc(sizeof(struct my_struct *))` allocates the wrong amount of space. You could avoid this by writing `my_arr[i] = malloc(sizeof *my_arr[i]);` – M.M Oct 23 '15 at 05:13
  • You `free` `my_arr[0]`, then immediately try to `memcpy` into the memory just freed. I'm guessing you meant to copy to `&my_arr[0]` from `&my_arr[1]`; the `&` are important (as is using `memmove` for overlapping `src`/`dst`). – ShadowRanger Oct 23 '15 at 05:35

2 Answers2

1
free(my_arr[0]);
memcpy(my_arr[0], my_arr[1], sizeof(struct my_struct *) * 9);

This is problem , you first free(my_arr[0]) and then copy my_arr[1] at address it points to .

You are not supposed to access memory after freeing it . And also specified in manpage

[...]The memory areas must not overlap. Use memmove if the memory areas do overlap.

again you do this -

   my_arr[9] = (struct my_struct *) malloc(sizeof(struct my_struct *));

thus , loosing reference to previous memory block it was pointing to .

ameyCU
  • 16,489
  • 2
  • 26
  • 41
1

This code works and I cleaned it up a bit:

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

struct my_struct {
  int a;
  int b;
};
int main(){
  const int structsz=sizeof(struct my_struct);
  struct my_struct **my_arr=malloc(10 * structsz);
  int i;
  printf("Before\n");
  for (i = 0; i < 10; i++){
    my_arr[i]=malloc(structsz);
    my_arr[i]->a=20+i;
    my_arr[i]->b=10+i;
    printf("i=%d  a=%d, b=%d\n",i,my_arr[i]->a,my_arr[i]->b);
  }
  free(my_arr[9]);
  my_arr[9]=malloc(structsz);
  memcpy(my_arr[9], my_arr[1], structsz); //make 1st struct in array equal the 9th
  free(my_arr[8]);
  my_arr[8]=malloc(structsz);
  memcpy(my_arr[8], my_arr[2], structsz); //make 2st struct in array equal the 8th
  printf("After\n");
  for (i = 0; i < 10; ++i) {
    printf("i=%d  a=%d, b=%d\n",i,my_arr[i]->a,my_arr[i]->b);
    free(my_arr[i]);
  }
  free(my_arr);
  return 0;
}

The reason why the third parameter of memcpy must be the same as the size of the structure is because both pointers in memcpy are the type of struct.

If the 3rd parameter is too large, then you can run into segmentation faults because you could try to copy memory that you're not allowed to access, or at best, you could be modifying other structs in your program.

If the 3rd parameter is too small, then you could receive invalid or insufficient data.

Mike -- No longer here
  • 2,064
  • 1
  • 15
  • 37
  • But wouldn't destination and source overlap in `memcpy` ? – ameyCU Oct 23 '15 at 05:38
  • my_arr has 10 usable elements. Each element has a pointer to its own memory as defined by the `malloc()` in the first for loop. but if I did `datatype* something=malloc(xxx);memcpy(datatype[n],datatype[n2],size);` then it would overlap because the first memory allocation isn't a double pointer. – Mike -- No longer here Oct 23 '15 at 05:42
  • In my comment, `datatype[n]` means the nth position from the beginning of where `datatype` address is. `*datatype = datatype[0]` `*(datatype+1) = datatype[1]` and so on. – Mike -- No longer here Oct 23 '15 at 05:50
  • No ,no I got that , therefore removed previous comment – ameyCU Oct 23 '15 at 05:51