Max Heap segmentation fault

Question

Hello dear computer lover. I have once again a problem.

I am supposed to program a Max-Heap. The heap.h and main.c are pre-set and should be correct.

I will now implement 5 functions:

Insert_heap
Heapify
Heap_create
Free_heap
Extract_max_heap

So far so good. The program compiles without errors. Valgrind however finds memory errors:

 "Invalid read / write of size 8" "use of uninitialized value"

I can not find the memory errors. In the follow the 5 functions written by me. I apologize for the structure.

Code:

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

#include "heap.h"

#define MAX_HEAP_SIZE 400
#define MAX_LINE_SIZE 100



int left(int i) {
    i = 2 * i;
    return i;
}

int right(int i) {
    i = 2 * i + 1;
    return i;
}

int parent(int i) {
    i = i / 2;
    return i;
}

heap* heap_create(size_t capacity) {
    heap* heap;
    heap = malloc(sizeof(heap));
    if(heap == NULL) {
        printf("Kein Speicher vorhanden\n");
        return NULL;
    }
    heap->size = 0;
    heap->capacity = capacity;
    heap->elements = malloc(capacity * sizeof(int));
    if(heap->elements == NULL) {
        printf("Kein Speicher vorhanden\n");
        return NULL;
    }
    return heap;

}


void heapify(heap* h, size_t i) {
    size_t links = left(i);
    size_t rechts = right(i);
    size_t largest;



    if(links <= h->size) {
        if(h->elements[(links)-1]>h->elements[i-1]) {
            largest = links;
        }
        else if(h->elements[(links)-1]<h->elements[i-1])
        largest = i;
    }


    if(rechts <= h->size && h->elements[rechts-1]>h->elements[i]) {
        largest = rechts;
    }

    if(largest != i) {
        size_t swap = h->elements[i];
        h->elements[i] = h->elements[largest];
        h->elements[largest] = swap;
        heapify(h, largest);
    }

}

int heap_extract_max(heap* h) {
    if (h->size < 1) {
        printf("Kein Element vorhanden!\n");
        return -1;
    }
    int max = h->elements[0];
    for(int i = 1; i < ((h->size)-1); i++) {
        h->elements[i-1] = h->elements[i];
    }
    h->size = ((h->size)-1);
    heapify(h, 1);
    return max;




}

int heap_insert(heap* h, int key) {


    if (h->size < (h->capacity)) {                                
        h->size = ((h->size)+1);                                      
        int i = h->size;

        if(i == 1) {
            h->elements[i-1] = key;
        }
        if(1 < i && i <= ((h->capacity))){
            if(h->elements[(parent(i))-1] >= key) {
                h->elements[i-1] = key;
            }
            else if(h->elements[(parent(i)-1)] < key) {
                h->elements[i-1] = h->elements[(parent(i)-1)];
                i = parent(i);
                h->elements[i-1] = key;
                heapify(h,h->elements[i-1]);

            }
        }


    }
    else if(h->size >= h->capacity) {
        return -1;
    }
    return 0;
}

void heap_free(heap* h) {
    free(h->elements);
    free(h);
}

valgrind:

==5080== Memcheck, a memory error detector
==5080== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==5080== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==5080== Command: ./a1
==5080== 
==5080== Invalid write of size 8
==5080==    at 0x4008AD: heap_create (introprog_maxheap.c:56)
==5080==    by 0x400F11: main (main_maxheap.c:73)
==5080==  Address 0x5203048 is 0 bytes after a block of size 8 alloc'd
==5080==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5080==    by 0x40088C: heap_create (introprog_maxheap.c:51)
==5080==    by 0x400F11: main (main_maxheap.c:73)
==5080== 
==5080== Invalid write of size 8
==5080==    at 0x4008BD: heap_create (introprog_maxheap.c:57)
==5080==    by 0x400F11: main (main_maxheap.c:73)
==5080==  Address 0x5203050 is 8 bytes after a block of size 8 alloc'd
==5080==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5080==    by 0x40088C: heap_create (introprog_maxheap.c:51)
==5080==    by 0x400F11: main (main_maxheap.c:73)
==5080== 


#######################
Boarding-Administration
Verfügbare Eingaben:
 <Zahl> Verfügbare Sitzreihe eintragen.
 n  Nächste zu belegende Sitzreihe erhalten.
 q  Programm beenden.

> 12

==5080== Invalid read of size 8
==5080==    at 0x400B45: heap_insert (introprog_maxheap.c:132)
==5080==    by 0x400FA1: main (main_maxheap.c:92)
==5080==  Address 0x5203048 is 0 bytes after a block of size 8 alloc'd
==5080==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5080==    by 0x40088C: heap_create (introprog_maxheap.c:51)
==5080==    by 0x400F11: main (main_maxheap.c:73)
==5080== 
==5080== Invalid read of size 8
==5080==    at 0x400B4D: heap_insert (introprog_maxheap.c:132)
==5080==    by 0x400FA1: main (main_maxheap.c:92)
==5080==  Address 0x5203050 is 8 bytes after a block of size 8 alloc'd
==5080==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5080==    by 0x40088C: heap_create (introprog_maxheap.c:51)
==5080==    by 0x400F11: main (main_maxheap.c:73)
==5080== 
==5080== Invalid read of size 8
==5080==    at 0x400B5E: heap_insert (introprog_maxheap.c:133)
==5080==    by 0x400FA1: main (main_maxheap.c:92)
==5080==  Address 0x5203048 is 0 bytes after a block of size 8 alloc'd
==5080==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5080==    by 0x40088C: heap_create (introprog_maxheap.c:51)
==5080==    by 0x400F11: main (main_maxheap.c:73)
==5080== 
==5080== Invalid write of size 8
==5080==    at 0x400B6A: heap_insert (introprog_maxheap.c:133)
==5080==    by 0x400FA1: main (main_maxheap.c:92)
==5080==  Address 0x5203048 is 0 bytes after a block of size 8 alloc'd
==5080==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5080==    by 0x40088C: heap_create (introprog_maxheap.c:51)
==5080==    by 0x400F11: main (main_maxheap.c:73)
==5080== 
==5080== Invalid read of size 8
==5080==    at 0x400B72: heap_insert (introprog_maxheap.c:134)
==5080==    by 0x400FA1: main (main_maxheap.c:92)
==5080==  Address 0x5203048 is 0 bytes after a block of size 8 alloc'd
==5080==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5080==    by 0x40088C: heap_create (introprog_maxheap.c:51)
==5080==    by 0x400F11: main (main_maxheap.c:73)
==5080== 

> 33

==5080== Invalid read of size 8
==5080==    at 0x400BB0: heap_insert (introprog_maxheap.c:139)
==5080==    by 0x400FA1: main (main_maxheap.c:92)
==5080==  Address 0x5203050 is 8 bytes after a block of size 8 alloc'd
==5080==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5080==    by 0x40088C: heap_create (introprog_maxheap.c:51)
==5080==    by 0x400F11: main (main_maxheap.c:73)
==5080== 
==5080== Invalid read of size 8
==5080==    at 0x400934: heapify (introprog_maxheap.c:80)
==5080==    by 0x400CBD: heap_insert (introprog_maxheap.c:147)
==5080==    by 0x400FA1: main (main_maxheap.c:92)
==5080==  Address 0x5203048 is 0 bytes after a block of size 8 alloc'd
==5080==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5080==    by 0x40088C: heap_create (introprog_maxheap.c:51)
==5080==    by 0x400F11: main (main_maxheap.c:73)
==5080== 
==5080== Invalid read of size 8
==5080==    at 0x4009BC: heapify (introprog_maxheap.c:89)
==5080==    by 0x400CBD: heap_insert (introprog_maxheap.c:147)
==5080==    by 0x400FA1: main (main_maxheap.c:92)
==5080==  Address 0x5203048 is 0 bytes after a block of size 8 alloc'd
==5080==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5080==    by 0x40088C: heap_create (introprog_maxheap.c:51)
==5080==    by 0x400F11: main (main_maxheap.c:73)
==5080== 
==5080== Conditional jump or move depends on uninitialised value(s)
==5080==    at 0x400A06: heapify (introprog_maxheap.c:93)
==5080==    by 0x400CBD: heap_insert (introprog_maxheap.c:147)
==5080==    by 0x400FA1: main (main_maxheap.c:92)
==5080== 
==5080== Use of uninitialised value of size 8
==5080==    at 0x400A46: heapify (introprog_maxheap.c:95)
==5080==    by 0x400CBD: heap_insert (introprog_maxheap.c:147)
==5080==    by 0x400FA1: main (main_maxheap.c:92)
==5080== 
==5080== Use of uninitialised value of size 8
==5080==    at 0x400A60: heapify (introprog_maxheap.c:96)
==5080==    by 0x400CBD: heap_insert (introprog_maxheap.c:147)
==5080==    by 0x400FA1: main (main_maxheap.c:92)
==5080== 
==5080== Invalid read of size 8
==5080==    at 0x400934: heapify (introprog_maxheap.c:80)
==5080==    by 0x400A74: heapify (introprog_maxheap.c:97)
==5080==    by 0x400CBD: heap_insert (introprog_maxheap.c:147)
==5080==    by 0x400FA1: main (main_maxheap.c:92)
==5080==  Address 0x5203048 is 0 bytes after a block of size 8 alloc'd
==5080==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5080==    by 0x40088C: heap_create (introprog_maxheap.c:51)
==5080==    by 0x400F11: main (main_maxheap.c:73)
==5080== 
==5080== Conditional jump or move depends on uninitialised value(s)
==5080==    at 0x40093C: heapify (introprog_maxheap.c:80)
==5080==    by 0x400A74: heapify (introprog_maxheap.c:97)
==5080==    by 0x400CBD: heap_insert (introprog_maxheap.c:147)
==5080==    by 0x400FA1: main (main_maxheap.c:92)
==5080== 
==5080== Invalid read of size 8
==5080==    at 0x4009BC: heapify (introprog_maxheap.c:89)
==5080==    by 0x400A74: heapify (introprog_maxheap.c:97)
==5080==    by 0x400CBD: heap_insert (introprog_maxheap.c:147)
==5080==    by 0x400FA1: main (main_maxheap.c:92)
==5080==  Address 0x5203048 is 0 bytes after a block of size 8 alloc'd
==5080==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5080==    by 0x40088C: heap_create (introprog_maxheap.c:51)
==5080==    by 0x400F11: main (main_maxheap.c:73)
==5080== 
==5080== Conditional jump or move depends on uninitialised value(s)
==5080==    at 0x4009C4: heapify (introprog_maxheap.c:89)
==5080==    by 0x400A74: heapify (introprog_maxheap.c:97)
==5080==    by 0x400CBD: heap_insert (introprog_maxheap.c:147)
==5080==    by 0x400FA1: main (main_maxheap.c:92)
==5080== 
==5080== Conditional jump or move depends on uninitialised value(s)
==5080==    at 0x400A06: heapify (introprog_maxheap.c:93)
==5080==    by 0x400A74: heapify (introprog_maxheap.c:97)
==5080==    by 0x400CBD: heap_insert (introprog_maxheap.c:147)
==5080==    by 0x400FA1: main (main_maxheap.c:92)
==5080== 
==5080== Use of uninitialised value of size 8
==5080==    at 0x400A1A: heapify (introprog_maxheap.c:94)
==5080==    by 0x400A74: heapify (introprog_maxheap.c:97)
==5080==    by 0x400CBD: heap_insert (introprog_maxheap.c:147)
==5080==    by 0x400FA1: main (main_maxheap.c:92)
==5080== 
==5080== Use of uninitialised value of size 8
==5080==    at 0x400A46: heapify (introprog_maxheap.c:95)
==5080==    by 0x400A74: heapify (introprog_maxheap.c:97)
==5080==    by 0x400CBD: heap_insert (introprog_maxheap.c:147)
==5080==    by 0x400FA1: main (main_maxheap.c:92)
==5080== 
==5080== Invalid read of size 4
==5080==    at 0x400A46: heapify (introprog_maxheap.c:95)
==5080==    by 0x400A74: heapify (introprog_maxheap.c:97)
==5080==    by 0x400CBD: heap_insert (introprog_maxheap.c:147)
==5080==    by 0x400FA1: main (main_maxheap.c:92)
==5080==  Address 0x4001202d90 is not stack'd, malloc'd or (recently) free'd
==5080== 
==5080== 
==5080== Process terminating with default action of signal 11 (SIGSEGV)
==5080==  Access not within mapped region at address 0x4001202D90
==5080==    at 0x400A46: heapify (introprog_maxheap.c:95)
==5080==    by 0x400A74: heapify (introprog_maxheap.c:97)
==5080==    by 0x400CBD: heap_insert (introprog_maxheap.c:147)
==5080==    by 0x400FA1: main (main_maxheap.c:92)
==5080==  If you believe this happened as a result of a stack
==5080==  overflow in your program's main thread (unlikely but
==5080==  possible), you can try to increase the size of the
==5080==  main thread stack using the --main-stacksize= flag.
==5080==  The main thread stack size used in this run was 8388608.
==5080== 
==5080== HEAP SUMMARY:
==5080==     in use at exit: 1,608 bytes in 2 blocks
==5080==   total heap usage: 4 allocs, 2 frees, 3,656 bytes allocated
==5080== 
==5080== LEAK SUMMARY:
==5080==    definitely lost: 0 bytes in 0 blocks
==5080==    indirectly lost: 0 bytes in 0 blocks
==5080==      possibly lost: 0 bytes in 0 blocks
==5080==    still reachable: 1,608 bytes in 2 blocks
==5080==         suppressed: 0 bytes in 0 blocks
==5080== Rerun with --leak-check=full to see details of leaked memory
==5080== 
==5080== For counts of detected and suppressed errors, rerun with: -v
==5080== Use --track-origins=yes to see where uninitialised values come from
==5080== ERROR SUMMARY: 26 errors from 21 contexts (suppressed: 0 from 0)
Segmentation fault

Unrelated, but instead of doing e.g. `i = 2 * i;` directly followed by `return i;`, why not simply do `return 2 * i;`? — Some programmer dude, Jan 27 '17 at 18:35
As for your problem, start by building a debug version of your program (if you use `gcc` then add the `-g` flag). Then run with Valgrind again. Now Valgrind will be able to tell you *where* it finds the problem. If you don't understand it, then edit your question to include the Valgrind output, and point out where in the code you show the errors are (using comments on those lines) — Some programmer dude, Jan 27 '17 at 18:42
Please *edit your question* instead of attempting to post it as a comment. — Some programmer dude, Jan 27 '17 at 18:47
the problem is at the function heapify and heap-insert...but i dont know where to find. of course i have the columns from valgrind but i did not know where's the mistake — CL89, Jan 27 '17 at 18:50
`heap* heap; heap = malloc(sizeof(heap));` Try `-Wshadow` and see if you can spot the problem. — EOF, Jan 27 '17 at 18:57
The line numbers in valgrind don't line up with the line numbers in your code. It makes it difficult to reconcile the two. Could you run valgrind on the code you posted? I'd run it myself, but it's currently busted on MacOS. Also, can we see heap.h? — Schwern, Jan 27 '17 at 21:16
`heap = malloc(sizeof(heap));` --> `heap = malloc(sizeof *heap);` Also avoid using a type/struct name the same as an object name. — chux - Reinstate Monica, Jan 27 '17 at 21:20
this line: `heap->elements = malloc(capacity * sizeof(int));` is saying that the field: `elements` is a pointer to an array of `int` values BUT this line: `size_t swap = h->elements[i];` is saying that the field: `elements` is a pointer to an array of `size_t` values. There are a LOT of such discrepancies in the posted code. When compiling, always enable all the warnings, then fix those warnings. (for `gcc`, at a minimum use: `-Wall -Wextra -pedantic -Wconversion -std=gnu99` ) — user3629249, Jan 28 '17 at 19:38
from the code, it is expected that 1) the `heap` struct is defined in the `heap.h` file and that file also contains ALL the function prototypes. This is a very bad idea because: 1) a calling function should NEVER be calling several of the functions in the posted code 2) exposes the implementation of the data to the outside world. Suggest 1) functions that will never be called have the `static` modifier added and removed from the header file 2) move the definition of the `heap` struct to the posted code, — user3629249, Jan 28 '17 at 19:46
the two definitions: `#define MAX_HEAP_SIZE 400` and `#define MAX_LINE_SIZE 100` are not used in the posted code, so should be deleted — user3629249, Jan 28 '17 at 19:56

score 1 · Answer 1 · answered Jan 27 '17 at 21:24

Take a close look at these lines in heap_create:

heap* heap;
heap = malloc(sizeof(heap));

Is the sizeof operator operating on the variable named heap, or the type named heap? It's not clear just by looking, but based on the valgrind output it looks like it's referring to the variable. It reports that 8 bytes were allocated, which is likely the size of a pointer (i.e. the heap variable) and not the type.

Because of this, it's not a good idea to have a variable with the same name as a type. So change the name of the variable heap to something like heap_top or just h to avoid ambiguity. Then you should be allocating the proper amount of memory.

Max Heap segmentation fault

1 Answers1