3

I am writing a simple program to make sure I fully understand how pointers in C work as I go through Harvard's CS50. Here is the code:

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

int main(void)
{
    int *a = malloc(sizeof(int));
    *a = 5;
    printf("the address of pointer a is: %p\n", &a);
    printf("the address that a is pointing to is: %p\n", a);
    printf("the contents of a are: %i\n", *a);

    printf("please enter a new value for a: \n");
    scanf("%i", a);
    printf("the address of pointer a is: %p\n", &a);
    printf("the address that a is pointing to is: %p\n", a);
    printf("the contents of a are: %i\n", *a);

    int *b = malloc(sizeof(int));
    *b = 7;
    printf("the address of pointer b is: %p\n", &b);
    printf("the address that b is pointing to is: %p\n", b);
    printf("the contents of b are: %i\n", *b);

    a = b;
    printf("setting a = b\n");
    printf("the address of pointer a is: %p\n", &a);
    printf("the address that a is pointing to is: %p\n", a);
    printf("the contents of a are: %i\n", *a);

    free(a);
    free(b);
}

It compiles w/o issues, but when executed, I get the following error: "* Error in `./address': double free or corruption (fasttop): 0x00000000018b7030 * Aborted"

This problem goes away if I get rid of either the free(a) or free(b) statements, however valgrind reveals a memory leak: "==9626== 4 bytes in 1 blocks are definitely lost in loss record 1 of 1 ==9626== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==9626== by 0x4206AD: main (address.c:6)"

I've looked through the other questions here and other websites that mention double free corruption, but still can't figure out quite what the issue is... I have a feeling that the answer is simple, and the issue probably lies in the statement "a = b", however I don't really get why I wouldn't be able to have one pointer point to a memory location that another one is pointing to, and then free the memory taken by both pointers...

Matvey
  • 335
  • 4
  • 18
  • 1
    Maybe instead of the assignment `a = b;` you meant `*a = *b;` — these do very different things, and the second notation (with the `*`'s) won't cause a problem. It assigns the value that `b` points at to the memory that `a` points at. – Jonathan Leffler Sep 28 '18 at 22:50
  • regarding: `It compiles w/o issues` This is not true. When compiling, always enable the warnings, then fix those warnings. ( for `gcc`, at a minimum use: `-Wall -Wextra -pedantic -Wconversion -std=gnu17` ) Note, other compilers have different options to perform the same functionality. – user3629249 Sep 28 '18 at 23:13
  • does it cleanly compile? No. The compiler outputs warnings for 2 out of every 3 calls to `printf()` This (probably) will not stop the code from working, but it sure isn't code with no problems – user3629249 Sep 28 '18 at 23:17
  • OT: When calling any of the heap allocation functions: `malloc` `calloc` `realloc`, always check (!=NULL) the returned value to assure the operation was successful. – user3629249 Sep 28 '18 at 23:20
  • You are NOT freeing memory "taken by pointers". You are freeing memory *pointed by* pointers. Since two pointers point to the same memory, you naturally end up with double free. – AnT stands with Russia Sep 28 '18 at 23:58
  • Awesome, thank you guys, this is a lot clearer now! @user3629249 I'm yet to learn how to use gcc properly since the CS50 class uses a "make" function created by the professor/staff for compiling. With the parameters you suggested, I get "gcc: error: unrecognized command line option ‘-std=gnu17’", but when I remove that one, I can see the warnings you mentioned. It'll take me some time to dig through those! – Matvey Sep 29 '18 at 16:09
  • suggest changing: `-std=gnu17` to `-std=gnu11` as the C17 standard is not available everywhere (yet) – user3629249 Sep 29 '18 at 17:03
  • here is a (somewhat dated) link to the `gcc` command line options and what they do. [gcc](https://linux.die.net/man/1/gcc) – user3629249 Sep 29 '18 at 17:11

3 Answers3

6

You're doing this:

 a = b; // a now points to the same thing as b does

...

free(a);
free(b);

... which is semantically equivalent to

free(b);
free(b);

That's the double free - b gets free'd twice - and that is disallowed in C.

For your memory leaking problem:

When you're setting a = b you're losing the original value of a. a was a pointer to the memory that you allocated with int *a = malloc(sizeof(int)); that is now lost. You need to save that address and pass it to free() before exit, if you want valgrind to stop complaining about leaks.

For each address that malloc() returns (except zero... ^_^), you should call free() with that very same address as argument.

Morten Jensen
  • 5,818
  • 3
  • 43
  • 55
  • 1
    Makes sense, thank you! Creating a temporary variable for storing the original value of a and then freeing that fixes the memory leak. – Matvey Sep 29 '18 at 16:10
  • @Matvey my pleasure :) Feel free to accept my answer if you want to give me some credit – Morten Jensen Oct 01 '18 at 23:58
4

Let's reduce your code to the bare minimum:

#include <stdlib.h>

int main(void)
{
    int *a = malloc(sizeof(int));
    int *b = malloc(sizeof(int));

    a = b; // after the assignment the pointer value previously held by a is gone.
           // both pointers point to the same memory. The memory previously pointed
           // to by a can no longer be free()d since its address is gone.

    free(a); // free the memory pointed to by b (a is a copy of b) once
    free(b); // free the memory pointed to by b twice --> boom.
}
Swordfish
  • 12,971
  • 3
  • 21
  • 43
1

When you do this:

 a = b;

You are effectively making a and b point to the same address (a.k.a. be the same pointer). So this is what causes the error:

free(a); // Frees the location where b points to
free(b);
yorodm
  • 4,359
  • 24
  • 32