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

[Franco Martelli]

On 16/12/24 at 16:58, Greg Wooledge wrote:
On Mon, Dec 16, 2024 at 16:05:26 +0100, Franco Martelli wrote:
void add_element( unsigned int i )
{
        DIGIT *p;
        /* If the first element (the head) has not been
        * created, create it now.
        */
        if ( head == NULL )
        {
                head = last = (DIGIT *) malloc( sizeof ( DIGIT ) );
                head->dgt = last->dgt = i;
                head->next = head->prev = last->next = last->prev = NULL;
                return;
        }
        /* Otherwise, find the last element in the list */
        for (p = head; p->next != NULL; p = p->next)
                ; /* null statement */
        p->next = (DIGIT *) malloc( sizeof ( DIGIT ) );
        p->next->prev = p;
        p->next->dgt = i;
        p->next->next = NULL;
        last = p->next;
}

If you're already keeping a "last" pointer which points to the end of
the linked list, you don't need that for loop to search for the end of
the list every time.

That's got nothing to do with memory leaks.  Just an observation.

Right, thank you

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

I think you might have an off-by-one error in this function.  You stop
the for loop when p->next == NULL, which means you never enter the body
during the time when p == last.  Which means you never free the
second-to-last element in the list.

It is p->prev not p->next, by doing so free() don't apply for the "last" 
element so I've to free apart


Given a list of 5 items, you will free items 1, 2, 3 (during the loop)
and 5 (after the loop), but not 4.

OK I'll try some test removing the "if statement"

Unless I've misread something.  You should definitely double-check me.

Yes, done ;)

--
Franco Martelli