-1
votes

I tried to write a C-program to show a colleague the use of heap-memory. But when I am running valgrind, I am getting error-messages I don't understand. Maybe someone can point me to my mistake.

From my perspective everything looks correct. The program should only create a new node, each time the n button is pressed. l should give a list of all existing nodes. The nodes are organized as a doubly-linked list.

node is just a struct, featuring a next_ptr and a prev_ptr that store a ptr to the previous and next node in the list. push_back should create a new empty node, manipulate the old heads next_ptr and then set the old heads address as the prev_ptr.

Here is the code:

node_t * get_last_item_ptr(node_t * cur_item_ptr){
        while(!is_last_item(cur_item_ptr)) cur_item_ptr = cur_item_ptr->next_ptr;
        return cur_item_ptr;
}


node_t * return_new_node_ptr(){
        node_t * new_node_ptr = (node_t*) malloc(sizeof(node_t));
        printf("new Node created at 0x%X!\n",new_node_ptr);
        return new_node_ptr;
}
void push_back(node_t * head_of_list_ptr, node_t * new_item_ptr){
        node_t * current_last_ptr = get_last_item_ptr(head_of_list_ptr);
        current_last_ptr->next_ptr = new_item_ptr;
        current_last_ptr->next_ptr->prev_ptr = current_last_ptr;
        current_last_ptr->next_ptr->next_ptr = NULL;
}


int main(){
        head_ptr = (node_t*) malloc(sizeof(head_ptr));
        if (NULL == head_ptr) return 1;
        head_ptr->next_ptr = NULL;

        while(1){
                printf("n = new node; l = list nodes; q = quit: ");
                scanf(" %c",&action);
                if('n' == action)      push_back(head_ptr, return_new_node_ptr());
                else if('l' == action) list_item_ptrs_from(head_ptr);
                else if('q' == action) break;
                else printf("Keine Aktion für Buchstabe = %c!\n",action);
        }
        return 0;
}

Here are the valgrind messages:

==51== error calling PR_SET_PTRACER, vgdb might block
==51== Invalid write of size 8
==51==    at 0x1088EF: main (memory.c:55)
==51==  Address 0x522f048 is 0 bytes after a block of size 8 alloc'd
==51==    at 0x4C31B0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==51==    by 0x1088CA: main (memory.c:53)
==51==
n = new node; l = list nodes; q = quit: n
new Node created at 0x5231110!
==51== Invalid read of size 8
==51==    at 0x1087A9: is_last_item (memory.c:19)
==51==    by 0x1087E5: get_last_item_ptr (memory.c:24)
==51==    by 0x10883F: push_back (memory.c:35)
==51==    by 0x10895F: main (memory.c:63)
==51==  Address 0x522f048 is 0 bytes after a block of size 8 alloc'd
==51==    at 0x4C31B0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==51==    by 0x1088CA: main (memory.c:53)
==51==
==51== Invalid write of size 8
==51==    at 0x10884C: push_back (memory.c:36)
==51==    by 0x10895F: main (memory.c:63)
==51==  Address 0x522f048 is 0 bytes after a block of size 8 alloc'd
==51==    at 0x4C31B0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==51==    by 0x1088CA: main (memory.c:53)
==51==
==51== Invalid read of size 8
==51==    at 0x108854: push_back (memory.c:37)
==51==    by 0x10895F: main (memory.c:63)
==51==  Address 0x522f048 is 0 bytes after a block of size 8 alloc'd
==51==    at 0x4C31B0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==51==    by 0x1088CA: main (memory.c:53)
==51==
==51== Invalid read of size 8
==51==    at 0x108863: push_back (memory.c:38)
==51==    by 0x10895F: main (memory.c:63)
==51==  Address 0x522f048 is 0 bytes after a block of size 8 alloc'd
==51==    at 0x4C31B0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==51==    by 0x1088CA: main (memory.c:53)
1
Please show a full minimal reproducible example. Sooo what is inside is_last_item? What input do you give to your program? From the mesages, you just type n<enter>? How is head_ptr defined? head_ptr = (node_t*) malloc(sizeof(head_ptr)); That's no right, are you allocating size of a pointer? Don't you want to allocate size of the head, and assign a pointer to that memory to head?KamilCuk
This is clearly wrong: head_ptr = (node_t*) malloc(sizeof(head_ptr));. The name of head_ptr is nothing if not telling. It's a pointer. You're allocating the size of one pointer. You're not allocating the size fo what that pointer should point to. That should be head_ptr = malloc(sizeof *head_ptr);WhozCraig

1 Answers

1
votes

You allocated size of a pointer, not size of memory you want to pointer to. You want:

node_t *head_ptr = malloc(sizeof(*head_ptr));
// or:
node_t *head_ptr = malloc(sizeof(note_t));

Do not cast result of malloc.