-3

I have the code below in a main.c file:

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

struct mystruct {const char *name; int x;};

char *test_calloc_char_p(const char *char_p) {
  return (char *) calloc(1, (strlen(char_p) + 1) * sizeof(char));
}

int main(void) {
  struct mystruct *ms1;
  ms1 = (struct mystruct *) calloc(1, sizeof(struct mystruct));
  ms1->name = test_calloc_char_p("dlldffdl");
  ms1->x = 3;
  free(&ms1->name);
  free(ms1);
  return 0;
}

Which I compile with gcc main.c -o test and run with ./test, and I get:

free(): double free detected in tcache 2
Abortado

I don't understant, why? I should not release ms1->name? because when I comment this line I don't get the error, but why shouldn't I release something that I allocated?

PerduGames
  • 1,108
  • 8
  • 19
  • `free(&ms1->name); ` Why are you taking the address of `ms1->name`? – PiRocks Nov 20 '19 at 17:58
  • it should be `free(ms1->name);` because you want to free the memory that `ms1->name` is pointing to. – Pablo Nov 20 '19 at 17:59
  • 1
    Tip: `calloc(1, (strlen(char_p) + 1) * sizeof(char))` doesn't make much sense. That should be `calloc(strlen(char_p)+1, sizeof(char))` or `malloc((strlen(char_p) + 1) * sizeof(char))`. Except `sizeof(char)` is guaranteed to be 1, so that should be `calloc(strlen(char_p)+1, 1)` or `malloc(strlen(char_p)+1)`. Also, the cast the return value isn't needed and therefore dangerous. It should be omitted. – ikegami Nov 20 '19 at 18:06
  • Tip: You might find `strdup` useful – ikegami Nov 20 '19 at 18:07

2 Answers2

2

This is incorrect:

free(&ms1->name);

The address of ms1->name is part of the allocated memory assigned to ms1. it is not the allocation for the name member.

You assigned allocated memory to ms1->name, so that is what you should pass to free:

free((char *)ms1->name);

Note that the cast is needed because otherwise you would be passing a const pointer to a function expecting a non-const pointer. The cast can be removed by changing the name member from const char * to char *.

dbush
  • 205,898
  • 23
  • 218
  • 273
  • That's right, I had put the "&" because I got the "expected 'void *' but argument is of type 'const char *'" warning, and I read it quickly, and didn't even realize it was a convertion error. Do I need convert by using "const" or also "char"? – PerduGames Nov 20 '19 at 19:22
  • @PerduGames The error you got was because you were trying to pass a `const` pointer where a non-`const` pointer was expected, so a cast is needed. The best way to fix this would be to change `name` to `char *` since the memory returned is writable. – dbush Nov 20 '19 at 19:25
  • So would it be writable even if I use `const` on the `name` member? – PerduGames Nov 20 '19 at 19:29
  • @PerduGames The `const` means you can't write to it via the `name` member but you can through some other `char *` pointer. Regardless, you should remove the `const` qualifier. – dbush Nov 20 '19 at 19:31
  • True, I tested here, I can record through another pointer anyway, I will remove `const`, it would be nice to show this solution in your answer since it is the most correct. – PerduGames Nov 20 '19 at 19:36
1

&ms1->name points to the address of first field in your struct, i.e. it points to ms1 itself. so ms1, and &ms1->name point to the same address

the code should be

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

struct mystruct {const char *name; int x;};

int main(void) {
  struct mystruct *ms1;
  ms1 = (struct mystruct *) calloc(1, sizeof(struct mystruct));
  ms1->name = strdup("dlldffdl");
  ms1->x = 3;
  free(ms1->name);
  free(ms1);
  return 0;
}

also your function test_calloc_char_p duplicates functionality of strdup

Maxim Sagaydachny
  • 2,098
  • 3
  • 11
  • 22