How to allocate memory for a structure member properly?

Question

Question's pretty self-explanatory. I want to implement a function which creates a structure with dynamic array of initial capacity of.. initial_capacity and some properties inside.
Here's the code:

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

typedef struct IntVector {
        int *vector;
        unsigned int capacity;
        unsigned int size;
} IntVector;

IntVector *int_vector_new(size_t initial_capacity) {
        struct IntVector v = {calloc(initial_capacity, sizeof(int)),
                               initial_capacity, 0};
        IntVector* v_ptr = &v;
        return v_ptr;
}

And here's what valgrind --leak-check=full --show-leak-kinds=all --track-origins=yes -v ./debug says:

==1713== Memcheck, a memory error detector
==1713== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==1713== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==1713== Command: ./debug
==1713==
--1713-- Valgrind options:
--1713--    --leak-check=full
--1713--    --show-leak-kinds=all
--1713--    --track-origins=yes
--1713--    -v
--1713-- Contents of /proc/version:
--1713--   Linux version 4.4.0-18362-Microsoft (Microsoft@Microsoft.com) (gcc version 5.4.0 (GCC) ) #476-Microsoft Fri Nov 01 16:53:00 PST 2019
--1713--
--1713-- Arch and hwcaps: AMD64, LittleEndian, amd64-cx16-lzcnt-rdtscp-sse3-avx-avx2-bmi
--1713-- Page sizes: currently 4096, max supported 4096
--1713-- Valgrind library directory: /usr/lib/valgrind
--1713-- Reading syms from /home/altbrace/IntVector/debug
--1713-- Reading syms from /lib/x86_64-linux-gnu/ld-2.27.so
--1713--   Considering /lib/x86_64-linux-gnu/ld-2.27.so ..
--1713--   .. CRC mismatch (computed 1b7c895e wanted 2943108a)
--1713--   Considering /usr/lib/debug/lib/x86_64-linux-gnu/ld-2.27.so ..
--1713--   .. CRC is valid
--1713-- Reading syms from /usr/lib/valgrind/memcheck-amd64-linux
--1713--   Considering /usr/lib/valgrind/memcheck-amd64-linux ..
--1713--   .. CRC mismatch (computed 41ddb025 wanted 9972f546)
--1713--    object doesn't have a symbol table
--1713--    object doesn't have a dynamic symbol table
--1713-- Scheduler: using generic scheduler lock implementation.
--1713-- Reading suppressions file: /usr/lib/valgrind/default.supp
==1713== embedded gdbserver: reading from /tmp/vgdb-pipe-from-vgdb-to-1713-by-altbrace-on-???
==1713== embedded gdbserver: writing to   /tmp/vgdb-pipe-to-vgdb-from-1713-by-altbrace-on-???
==1713== embedded gdbserver: shared mem   /tmp/vgdb-pipe-shared-mem-vgdb-1713-by-altbrace-on-???
==1713==
==1713== TO CONTROL THIS PROCESS USING vgdb (which you probably
==1713== don't want to do, unless you know exactly what you're doing,
==1713== or are doing some strange experiment):
==1713==   /usr/lib/valgrind/../../bin/vgdb --pid=1713 ...command...
==1713==
==1713== TO DEBUG THIS PROCESS USING GDB: start GDB like this
==1713==   /path/to/gdb ./debug
==1713== and then give GDB the following command
==1713==   target remote | /usr/lib/valgrind/../../bin/vgdb --pid=1713
==1713== --pid is optional if only one valgrind process is running
==1713==
==1713== error calling PR_SET_PTRACER, vgdb might block
--1713-- REDIR: 0x401f2f0 (ld-linux-x86-64.so.2:strlen) redirected to 0x580608c1 (???)
--1713-- REDIR: 0x401f0d0 (ld-linux-x86-64.so.2:index) redirected to 0x580608db (???)
--1713-- Reading syms from /usr/lib/valgrind/vgpreload_core-amd64-linux.so
--1713--   Considering /usr/lib/valgrind/vgpreload_core-amd64-linux.so ..
--1713--   .. CRC mismatch (computed 50df1b30 wanted 4800a4cf)
--1713--    object doesn't have a symbol table
--1713-- Reading syms from /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so
--1713--   Considering /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so ..
--1713--   .. CRC mismatch (computed f893b962 wanted 95ee359e)
--1713--    object doesn't have a symbol table
==1713== WARNING: new redirection conflicts with existing -- ignoring it
--1713--     old: 0x0401f2f0 (strlen              ) R-> (0000.0) 0x580608c1 ???
--1713--     new: 0x0401f2f0 (strlen              ) R-> (2007.0) 0x04c32db0 strlen
--1713-- REDIR: 0x401d360 (ld-linux-x86-64.so.2:strcmp) redirected to 0x4c33ee0 (strcmp)
--1713-- REDIR: 0x401f830 (ld-linux-x86-64.so.2:mempcpy) redirected to 0x4c374f0 (mempcpy)
--1713-- Reading syms from /lib/x86_64-linux-gnu/libc-2.27.so
--1713--   Considering /lib/x86_64-linux-gnu/libc-2.27.so ..
--1713--   .. CRC mismatch (computed b1c74187 wanted 042cc048)
--1713--   Considering /usr/lib/debug/lib/x86_64-linux-gnu/libc-2.27.so ..
--1713--   .. CRC is valid
--1713-- REDIR: 0x4edac70 (libc.so.6:memmove) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4ed9d40 (libc.so.6:strncpy) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4edaf50 (libc.so.6:strcasecmp) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4ed9790 (libc.so.6:strcat) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4ed9d70 (libc.so.6:rindex) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4edc7c0 (libc.so.6:rawmemchr) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4edade0 (libc.so.6:mempcpy) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4edac10 (libc.so.6:bcmp) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4ed9d00 (libc.so.6:strncmp) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4ed9800 (libc.so.6:strcmp) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4edad40 (libc.so.6:memset) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4ef80f0 (libc.so.6:wcschr) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4ed9ca0 (libc.so.6:strnlen) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4ed9870 (libc.so.6:strcspn) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4edafa0 (libc.so.6:strncasecmp) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4ed9840 (libc.so.6:strcpy) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4edb0e0 (libc.so.6:memcpy@@GLIBC_2.14) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4ed9da0 (libc.so.6:strpbrk) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4ed97c0 (libc.so.6:index) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4ed9c70 (libc.so.6:strlen) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4ee46c0 (libc.so.6:memrchr) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4edaff0 (libc.so.6:strcasecmp_l) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4edabe0 (libc.so.6:memchr) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4ef8eb0 (libc.so.6:wcslen) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4eda050 (libc.so.6:strspn) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4edaf20 (libc.so.6:stpncpy) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4edaef0 (libc.so.6:stpcpy) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4edc7f0 (libc.so.6:strchrnul) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4edb040 (libc.so.6:strncasecmp_l) redirected to 0x4a2a6e0 (_vgnU_ifunc_wrapper)
--1713-- REDIR: 0x4fca3c0 (libc.so.6:__strrchr_avx2) redirected to 0x4c32730 (rindex)
--1713-- REDIR: 0x4ed6030 (libc.so.6:calloc) redirected to 0x4c31a70 (calloc)
--1713-- REDIR: 0x4ed3950 (libc.so.6:free) redirected to 0x4c30cd0 (free)
==1713==
==1713== HEAP SUMMARY:
==1713==     in use at exit: 20 bytes in 1 blocks
==1713==   total heap usage: 1 allocs, 0 frees, 20 bytes allocated
==1713==
==1713== Searching for pointers to 1 not-freed blocks
==1713== Checked 69,336 bytes
==1713==
==1713== 20 bytes in 1 blocks are definitely lost in loss record 1 of 1
==1713==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1713==    by 0x1086F2: int_vector_new (IntVector.c:13)
==1713==    by 0x1086BB: main (main.c:8)
==1713==
==1713== LEAK SUMMARY:
==1713==    definitely lost: 20 bytes in 1 blocks
==1713==    indirectly lost: 0 bytes in 0 blocks
==1713==      possibly lost: 0 bytes in 0 blocks
==1713==    still reachable: 0 bytes in 0 blocks
==1713==         suppressed: 0 bytes in 0 blocks
==1713==
==1713== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
==1713== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

I've been trying to fix this for three hours straight with no progress, as you can see.
Hope on your help.

How did you compile your program? Do you have a separate file somewhere with a main function? — Addison, Feb 26 '20 at 17:40
You can't return a pointer to a local variable, the variable is destroyed when the function returns. — Barmar, Feb 26 '20 at 17:42
What do you think happens to `struct IntVector v` after `int_vector_new()` returns? — Andrew Henle, Feb 26 '20 at 17:44
Detail: C defines `.vector, .capacity, ...` as _members_, not _fields_. — chux - Reinstate Monica, Feb 26 '20 at 19:19

Vlad from Moscow · Answer 1 · 2020-02-26T19:01:07.850

3

The function

IntVector *int_vector_new(size_t initial_capacity) {
        struct IntVector v = {calloc(initial_capacity, sizeof(int)),
                               initial_capacity, 0};
        IntVector* v_ptr = &v;
        return v_ptr;
}

returns a pointer to a local variable declared like

struct IntVector v

So after exiting the function the pointer has an invalid value because the pointed object is not alive any more.

Either allocate an object of the structure type dynamically and return pointer to the allocated memory. Or return the object v itself by value to the caller of the function.

For example

IntVector int_vector_new(size_t initial_capacity) {
        struct IntVector v = {calloc(initial_capacity, sizeof(int)),
                               initial_capacity, 0};
        return v;
}

or

IntVector * int_vector_new(size_t initial_capacity) {
        struct IntVector* v_ptr = malloc( sizeof( struct IntVentor ) );

        v_ptr->vector = calloc( initial_capacity, sizeof(int));
        v_ptr->capacity = initial_capacity;
        v_ptr->size = 0;
        return v_ptr;
}

You can add a check whether the memory was allocated successfully.

edited Feb 26 '20 at 19:01

answered Feb 26 '20 at 17:43

Vlad from Moscow

301,070
26
186
335

Or have the caller create the structure and pass a pointer to it, and have this function populate the members of that structure appropriately. – supercat Feb 26 '20 at 17:45
What does the "struct Inventor" stand for? GCC throws quite reasonable error. I took it as a typo and corrected to "struct IntVector", but this causes memory leak too, a bit distinct from what I get with my code, though. – altbrace Feb 26 '20 at 18:00
@altbrace It is a typo. There is no memory leak if you will not forgot to free data members. – Vlad from Moscow Feb 26 '20 at 19:01
Yeah, Addison explained it to me. Thanks to you too – altbrace Feb 26 '20 at 19:31

Addison · Accepted Answer · 2020-02-26T18:56:19.143

0

I don't know everything that may be going on because at the moment you don't have any code showing where this is called, but I see a big issue. You are returning a pointer to a value created in int_vector_new, so once that function returns the local variable pointed to (v) will be gone.

You could do allocate memory for v, like this: IntVector* v = malloc(sizeof(IntVector));.

Then set each field using the -> syntax. Like this:

IntVector* v = malloc(sizeof(IntVector));
v->vector = calloc(initial_capacity, sizeof(int));
v->capacity = initial_capacity;
v->size = 0;

return v;

Since valgrind complains about leaks, I'd guess you are not freeing vector later on in your code. Freeing that will fix the complaint about the leaked memory. And once you use malloc to allocate v, you will need to free any values that int_vector_new returns.

edited Feb 26 '20 at 18:56

answered Feb 26 '20 at 17:45

Addison

3,791
3
28
48

regarding the statements: `#include ` and `#include ` 1) the heap allocation functions, including `malloc()` are prototyped in the header `stdlib.h` so the header `malloc.h` is not needed (and should be removed) 2) nothing from the header file: `stddef.h` are being used. It is a very poor programming practice to include header files those contents are not used. Suggest removing that statement. – user3629249 Feb 26 '20 at 17:58
when compiling, always enable the warnings, then fix those warnings. ( for `gcc`, at a minimum use: `-Wall -Wextra -Wconversion -pedantic -std=gnu11` ) Note: other compilers use different options to produce the same results – user3629249 Feb 26 '20 at 18:00
How do I free structure fields? – altbrace Feb 26 '20 at 18:27
@altbrace If you have a `IntVector *` named `v`, you can do `free(v->capacity)`. And if you have a `IntVector`, it would be `free(v.capacity)`. Basically `->` is used to access struct fields when you have a pointer to a struct. `a->b` is the same as `(*a).b`. – Addison Feb 26 '20 at 18:29
But `capacity` is an integer, `free()` takes a pointer as an argument – altbrace Feb 26 '20 at 18:47
Did it like that and it compiled without any warnings `size_t* capacity_ptr = &(v->capacity); free(capacity_ptr);`. But when executed, throws `free(): invalid pointer` – altbrace Feb 26 '20 at 18:54
@altbrace Sorry, my bad! That was a typo, you need to be freeing `vector`, not `capacity`. Since `vector` was allocated with calloc/malloc/realloc, it needs to be freed. `free(v->vector)` should work. I've updated the answer accordingly. – Addison Feb 26 '20 at 18:57
Memory leak while allocating `IntVector* v` – altbrace Feb 26 '20 at 19:05
Just checked - despite what valgrind says (most possibly I simply don't know how to read it's output), it works! Thanks! – altbrace Feb 26 '20 at 19:28
@altbrace great! That last error is probably from not freeing the value you get back from `int_vector_new`. You'll need to free that after you free the vector field on it. – Addison Feb 26 '20 at 19:32
1

I'll figure it out later. Just've implemented a new function that sets values to check if it works. 4140 bytes leaked xD. – altbrace Feb 26 '20 at 19:39

How to allocate memory for a structure member properly?

2 Answers2