1

I got a segfault error while executing this code. It prints the largest of 5 numbers, and those numbers are stored in heap-memory.

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

int main() {
    int *ptr = (int *)malloc(5 * sizeof(int));
    *ptr = 5;
    *(ptr + 1) = 7;
    *(ptr + 2) = 2;
    *(ptr + 3) = 9;
    *(ptr + 4) = 8;

    int *ptr_max = (int *)malloc(sizeof(int));
    *ptr_max = 0;

    for (int i = 0; i < 5; i++) {
        if (*ptr_max < *ptr) {
            *ptr_max = *ptr;
            ptr++;
        } else
            ptr++;
    }
    printf("%d\n", *ptr_max);
    free(ptr);
    free(ptr_max);
    return 0;
}

I want to know why exactly I got this error from the above code. Please can anyone explain it to me?

Darth-CodeX
  • 2,166
  • 1
  • 6
  • 23
alper ortac
  • 49
  • 1
  • 6
  • 2
    [don't cast malloc](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) – Barmar May 18 '22 at 18:14
  • 3
    You are `free`-ing not the same `ptr` you have allocated. You have incremented it 5 times. – Eugene Sh. May 18 '22 at 18:15
  • @EugeneSh. Why don't you consider that worth an answer? – Yunnosch May 18 '22 at 18:18
  • @Yunnosch I don't think it is going to be any useful as a QA.. – Eugene Sh. May 18 '22 at 18:19
  • @Eugene Sh. i know stg is wrong with my code but i needed to be sure which wrong thing exactly giving this error thank you. – alper ortac May 18 '22 at 18:24
  • @alperortac If it wasn't clear from my initial comment - you *must* pass to `free` the same value you have obtained from `malloc`. But you are passing it `ptr` which was incremented (`ptr++`) several times after it was returned from `malloc`. – Eugene Sh. May 18 '22 at 18:26

1 Answers1

1

The real problem lies when you are free()-ing the ptr. Once, you increment a pointer its address jumps to next address allocated by malloc(). Always make sure that *ptr is same as ptr[0]. So, to fix this issue, you can decrement the ptr by 5, or create a copied pointer.

Example of address given to free(), they are not pointing to the same memory block:

Before decrementing 0x213f2b4
After decrementing 0x213f2a0

The reason for decrementing by 5, is the difference between these two hexadecimal values which is 20, same as sizeof(int) * 5.

ptr -= 5;

OR

You can create a copy of your pointer and then perform operations on that copied one:

int *my_copied_ptr = ptr; // you don't need to free this pointer

Then, free() them:

free(ptr);
free(ptr_max);

Now, to avoid these mistakes further in a large code bases, try using [] operator like this:

ptr[0] = 5;
ptr[1] = 7;
ptr[2] = 2;
ptr[3] = 9;
ptr[4] = 8;
Darth-CodeX
  • 2,166
  • 1
  • 6
  • 23
  • The first approach seem to be quite fragile. In practice we usually save the original pointer, and working with a copy. – Eugene Sh. May 18 '22 at 18:45
  • The `ptr -= 5;` one. – Eugene Sh. May 18 '22 at 18:48
  • 1
    `int *saved_ptr = ptr;` before the loop then `free(saved_ptr);` at the end. – Barmar May 18 '22 at 18:51
  • Your method with `ptr -= 5;` works when you know that the pointer will always be incremented 5 times, like the example. But in most real applications, the increments will be more dynamic and you don't know how much to subtract. – Barmar May 18 '22 at 18:52
  • i did working version of this code. but as i told you before i needed to be sure what is exactly wrong with this one thank you too. – alper ortac May 18 '22 at 18:53
  • @alperortac I am not sure how to interpret your comment. Is the answer clear and is answering your question? – Eugene Sh. May 18 '22 at 18:57
  • 1
    It has already been explained that you must pass to `free` the same value you received from `malloc`. Your original program did not; the pointer was incremented. You *cannot* do `p = malloc(...); p++; free(p);`. That was the issue; I don't know how this can be made clearer... – printf May 18 '22 at 19:05