Re: OT: Possible memory leak in an exercise of a C handbook

[Jeffrey Walton <noloader@gmail.com>]

On Mon, Dec 16, 2024 at 2:22 PM Franco Martelli <martellif67@gmail.com> wrote:
>
> I'm doing the exercises of a C language handbook. I'm using Valgrind to
> check for memory leak since I use the malloc calls. In the past I was
> used to using "valkyrie" but sadly isn't available anymore for Bookworm
> (does anybody know for a replacement?).
>
> I suppose that Valgrind detects a memory leak for my source code, here
> its output:
>
> $ valgrind --track-origins=yes --leak-check=full -s ./a.out
> ==101348== Memcheck, a memory error detector
> ==101348== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
> ==101348== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright
> info
> ==101348== Command: ./a.out
> ==101348==
> 12345678
> ==101348==
> ==101348== HEAP SUMMARY:
> ==101348==     in use at exit: 24 bytes in 1 blocks
> ==101348==   total heap usage: 9 allocs, 8 frees, 1,216 bytes allocated
> ==101348==
> ==101348== 24 bytes in 1 blocks are definitely lost in loss record 1 of 1
> ==101348==    at 0x48407B4: malloc (in
> /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==101348==    by 0x10924B: add_element (in a.out)
> ==101348==    by 0x1093B3: main (in a.out)
> ==101348==
> ==101348== LEAK SUMMARY:
> ==101348==    definitely lost: 24 bytes in 1 blocks
> ==101348==    indirectly lost: 0 bytes in 0 blocks
> ==101348==      possibly lost: 0 bytes in 0 blocks
> ==101348==    still reachable: 0 bytes in 0 blocks
> ==101348==         suppressed: 0 bytes in 0 blocks
> ==101348==
> ==101348== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
>
> Is there a memory leak? What it sounds strange to me is that Valgrind
> reports: "total heap usage: 9 allocs, 8 frees, …" when for me the calls
> to "malloc" should be 8, not 9.

Yes.

> Is there any C guru here that can take a look to my source code in
> attachment?

Here's the problem:

void dealloc()
{
    for ( const DIGIT *p = first; p->next != NULL; p = p->next )
        if ( p->prev != NULL )
            free( p->prev );
    free( last );
}

You seem to be checking backwards (p->prev) but walking the list
forwards (p = p->next) at the same time.

Try something like this. It allows you to walk the list forward.

    void dealloc()
    {
        for ( const DIGIT *next, *p = head; p->next != NULL; )
            if ( p != NULL )
                next = p->next, free( p ), p = next;
        free( last );
    }

The use of 'next' stashes away the pointer so you can free 'p' and
still access the next pointer.

If you don't want to use the comma operator, then this:

    if ( p != NULL ) {
        next = p->next;
        free( p );
        p = next;
    }

> In order to avoid off topic messages in the future does
> anybody know where to ask for these topics?

Stack Overflow is usually a good place for programming questions.

Jeff