On Tue, Dec 17, 2024 at 2:39 PM Franco Martelli <martelli...@gmail.com> wrote:
>
> On 16/12/24 at 20:49, Jeffrey Walton wrote:
> > 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.
>
> Thanks, your code works, Valgrind says 0 errors and: "All heap blocks
> were freed -- no leaks are possible".
> GCC gives me a warning, so I've to remove the "const" modifier in the
> "for loop":
>
> $ gcc -Wall e09-01.c
> e09-01.c: In function ‘dealloc’:
> e09-01.c:57:47: warning: passing argument 1 of ‘free’ discards ‘const’
> qualifier from pointer target type [-Wdiscarded-qualifiers]
> 57 | next = p->next, free( p ), p = next;
> | ^
> In file included from e09-01.c:5:
> /usr/include/stdlib.h:568:25: note: expected ‘void *’ but argument is of
> type ‘const DIGIT *’ {aka ‘const struct digit *’}
> 568 | extern void free (void *__ptr) __THROW;
>
> after done this, all works nicely. Thanks again.
There may be one logic error in the code -- if you insert one item,
then you may double free the node because you free 'p' and then you
free 'last'.
I would rewrite the cleanup code like so:
void dealloc()
{
DIGIT *next, *p = head;
while( p )
next = p->next, free( p ), p = next;
}
Then add three test cases instead of one. Have a test case for 0
items, 1 item, and N items (like 8).
Jeff
