Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.debian.user > #275767 > unrolled thread

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

Back to article view | Back to linux.debian.user

Contents

  OT: Possible memory leak in an exercise of a C handbook Franco Martelli <martellif67@gmail.com> - 2024-12-16 16:10 +0100
    Re: OT: Possible memory leak in an exercise of a C handbook Michael Kjörling <c9bc136c6063@ewoof.net> - 2024-12-16 16:50 +0100
      Re: OT: Possible memory leak in an exercise of a C handbook Franco Martelli <martellif67@gmail.com> - 2024-12-16 17:30 +0100
        Re: OT: Possible memory leak in an exercise of a C handbook Michael Kjörling <c9bc136c6063@ewoof.net> - 2024-12-16 20:50 +0100
          Re: OT: Possible memory leak in an exercise of a C handbook Franco Martelli <martellif67@gmail.com> - 2024-12-16 22:00 +0100
    Re: OT: Possible memory leak in an exercise of a C handbook Greg Wooledge <greg@wooledge.org> - 2024-12-16 17:00 +0100
      Re: OT: Possible memory leak in an exercise of a C handbook Franco Martelli <martellif67@gmail.com> - 2024-12-16 17:40 +0100
        Re: OT: Possible memory leak in an exercise of a C handbook Greg Wooledge <greg@wooledge.org> - 2024-12-16 18:00 +0100
          Re: OT: Possible memory leak in an exercise of a C handbook Franco Martelli <martellif67@gmail.com> - 2024-12-16 21:20 +0100
    Re: OT: Possible memory leak in an exercise of a C handbook Charles Curley <charlescurley@charlescurley.com> - 2024-12-16 19:40 +0100
    Re: OT: Possible memory leak in an exercise of a C handbook Jeffrey Walton <noloader@gmail.com> - 2024-12-16 21:00 +0100
      Re: OT: Possible memory leak in an exercise of a C handbook Franco Martelli <martellif67@gmail.com> - 2024-12-16 22:40 +0100
        Re: OT: Possible memory leak in an exercise of a C handbook songbird <songbird@anthive.com> - 2024-12-17 05:00 +0100
        Re: OT: Possible memory leak in an exercise of a C handbook Anssi Saari <anssi.saari@debian-user.mail.kapsi.fi> - 2024-12-17 12:30 +0100
          Re: OT: Possible memory leak in an exercise of a C handbook Franco Martelli <martellif67@gmail.com> - 2024-12-17 15:30 +0100
            Re: OT: Possible memory leak in an exercise of a C handbook Anssi Saari <anssi.saari@debian-user.mail.kapsi.fi> - 2024-12-18 11:20 +0100
          Re: OT: Possible memory leak in an exercise of a C handbook Jean-François Bachelet <jfbachelet@free.fr> - 2024-12-18 05:50 +0100
      Re: OT: Possible memory leak in an exercise of a C handbook Franco Martelli <martellif67@gmail.com> - 2024-12-17 20:50 +0100
        Re: OT: Possible memory leak in an exercise of a C handbook Greg Wooledge <greg@wooledge.org> - 2024-12-17 22:20 +0100
          Re: OT: Possible memory leak in an exercise of a C handbook <tomas@tuxteam.de> - 2024-12-18 06:10 +0100
            Re: OT: Possible memory leak in an exercise of a C handbook Kevin Chadwick <m8il1ists@gmail.com> - 2024-12-18 11:50 +0100
              Re: OT: Possible memory leak in an exercise of a C handbook <tomas@tuxteam.de> - 2024-12-18 12:00 +0100
        Re: OT: Possible memory leak in an exercise of a C handbook Jeffrey Walton <noloader@gmail.com> - 2024-12-17 22:20 +0100
          Re: OT: Possible memory leak in an exercise of a C handbook Franco Martelli <martellif67@gmail.com> - 2024-12-18 17:00 +0100
            Re: OT: Possible memory leak in an exercise of a C handbook Jeffrey Walton <noloader@gmail.com> - 2024-12-18 17:20 +0100

Page 1 of 2  [1] 2  Next page →

#275767 — OT: Possible memory leak in an exercise of a C handbook

[Multipart message — attachments visible in raw view] — view raw

Hi,

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.

Is there any C guru here that can take a look to my source code in 
attachment? In order to avoid off topic messages in the future does 
anybody know where to ask for these topics?
TIA
-- 
Franco Martelli

[toc] | [next] | [standalone]

#275772

On 16 Dec 2024 16:05 +0100, from martellif67@gmail.com (Franco Martelli):
> 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.

Put in something to count the number of calls to malloc() and free()
respectively. Don't forget calls outside of loops.

This can be as simple as a `printf("+");` and `printf("-");`
respectively at each call site. Or put breakpoints at each (or at the
respective function entry point) and keep a manual count.

See how many times each is reached.

-- 
Michael Kjörling
🔗 https://michael.kjorling.se

[toc] | [prev] | [next] | [standalone]

#275778

On 16/12/24 at 16:43, Michael Kjörling wrote:
> On 16 Dec 2024 16:05 +0100, from martellif67@gmail.com (Franco Martelli):
>> 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.
> 
> Put in something to count the number of calls to malloc() and free()
> respectively. Don't forget calls outside of loops.

I did it, I've put a "cnt" variable in body of the "for loop" in main() 
and it results in 8 calls.
There isn't calls to malloc() or free() outside loops. What do you mean?
> 
> This can be as simple as a `printf("+");` and `printf("-");`
> respectively at each call site. Or put breakpoints at each (or at the
> respective function entry point) and keep a manual count.
> 
> See how many times each is reached.
> 
I don't know how to put breakpoints, I'm new to gdb :(

-- 
Franco Martelli

[toc] | [prev] | [next] | [standalone]

#275793

On 16 Dec 2024 17:21 +0100, from martellif67@gmail.com (Franco Martelli):
>> Put in something to count the number of calls to malloc() and free()
>> respectively. Don't forget calls outside of loops.
> 
> There isn't calls to malloc() or free() outside loops. What do you mean?

>From a quick re-glance through your code...

        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 ) );

and

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

certainly look to me like they're outside of loops.

-- 
Michael Kjörling
🔗 https://michael.kjorling.se

[toc] | [prev] | [next] | [standalone]

#275797

On 16/12/24 at 20:42, Michael Kjörling wrote:
> On 16 Dec 2024 17:21 +0100, from martellif67@gmail.com (Franco Martelli):
>>> Put in something to count the number of calls to malloc() and free()
>>> respectively. Don't forget calls outside of loops.
>>
>> There isn't calls to malloc() or free() outside loops. What do you mean?
> 
>>From a quick re-glance through your code...
> 
>          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 ) );

Yes, but those calls in add_element(int) happen because add_element(int) 
is called only by the "for loop" in main().
I realized what you suggest after reply to your email: place a 
printf("+") after every malloc(…) call and place a printf("-") after 
every free() call.

> 
> and
> 
>          for ( const DIGIT *p = head; p->next != NULL; p = p->next )
>                  if ( p->prev != NULL )
>                          free( p->prev );
> ==>     free( last );
> 
> certainly look to me like they're outside of loops.
> 
Yes, that call is outside the loop, but I realized after reply to your 
email of your suggestion.
Thank you very much for the tips!

-- 
Franco Martelli

[toc] | [prev] | [next] | [standalone]

#275773

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.

> 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.

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

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

[toc] | [prev] | [next] | [standalone]

#275779

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

[toc] | [prev] | [next] | [standalone]

#275783

On Mon, Dec 16, 2024 at 17:34:36 +0100, Franco Martelli wrote:
> > > 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"

No, it's not the if statement.  It's the for loop's iteration condition.

> > > 	for ( const DIGIT *p = head; p->next != NULL; p = p->next )

Your iteration condition is  p->next != NULL  so  you stop iterating as
soon as p->next == NULL.

Given this linked list:

  NULL <- [1] <-> [2] <-> [3] <-> [4] <-> [5] -> NULL
           ^                               ^
           head                            last

Your first loop iteration, when p == head, does nothing because of
the if statement.  That's fine.

The second iteration, when p points to [2], frees [1].

The third iteration, when p points to [3], frees [2].

The fourth iteration, when p points to [4], frees [3].

The fifth iteration, when p points to [5], NEVER HAPPENS, because
p->next == NULL at that point, and the for loop stops.  So [4] is
not freed.

After the loop, you free [5] which is where last points.

[toc] | [prev] | [next] | [standalone]

#275796

On 16/12/24 at 17:50, Greg Wooledge wrote:
> On Mon, Dec 16, 2024 at 17:34:36 +0100, Franco Martelli wrote:
>>>> 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"
> 
> No, it's not the if statement.  It's the for loop's iteration condition.
> 
>>>> 	for ( const DIGIT *p = head; p->next != NULL; p = p->next )
> 
> Your iteration condition is  p->next != NULL  so  you stop iterating as
> soon as p->next == NULL.
> 
> Given this linked list:
> 
>    NULL <- [1] <-> [2] <-> [3] <-> [4] <-> [5] -> NULL
>             ^                               ^
>             head                            last
> 
> Your first loop iteration, when p == head, does nothing because of
> the if statement.  That's fine.
> 
> The second iteration, when p points to [2], frees [1].
> 
> The third iteration, when p points to [3], frees [2].
> 
> The fourth iteration, when p points to [4], frees [3].
> 
> The fifth iteration, when p points to [5], NEVER HAPPENS, because
> p->next == NULL at that point, and the for loop stops.  So [4] is
> not freed.
> 
> After the loop, you free [5] which is where last points.
> 
You're correct! To verify I've added "free( last->prev );" to dealloc():

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

and it worked, now the output of Valgrind is:

$ valgrind --track-origins=yes --leak-check=full -s ./a.out
==114549== Memcheck, a memory error detector
==114549== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==114549== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright 
info
==114549== Command: ./a.out
==114549==
12345678
==114549==
==114549== HEAP SUMMARY:
==114549==     in use at exit: 0 bytes in 0 blocks
==114549==   total heap usage: 9 allocs, 9 frees, 1,216 bytes allocated
==114549==
==114549== All heap blocks were freed -- no leaks are possible
==114549==
==114549== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

I'll change the "for condition" thanks very much again

-- 
Franco Martelli

[toc] | [prev] | [next] | [standalone]

#275788

On Mon, 16 Dec 2024 16:05:26 +0100
Franco Martelli <martellif67@gmail.com> wrote:

> I'm doing the exercises of a C language handbook.

By all means do the exercises in your handbook as a learning
experience. After that, I have found very useful Roger Sessions,
Reusable Data Structures For C, Prentice Hall (1989).

-- 
Does anybody read signatures any more?

https://charlescurley.com
https://charlescurley.com/blog/

[toc] | [prev] | [next] | [standalone]

#275794

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

[toc] | [prev] | [next] | [standalone]

#275799

On 16/12/24 at 20:49, Jeffrey Walton wrote:
> 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;
>      }
> 

Thank you very much, I'll try that code tomorrow

>> 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.

I'd prefer a mailing-list instead, once finished all the exercises, I'd 
like to looking for somebody that he has my same handbook and to ask him 
for exchange the exercises for comparison purpose.
Does anybody know a mailing-list for C language questions?

-- 
Franco Martelli

[toc] | [prev] | [next] | [standalone]

#275806

Franco Martelli wrote:
...
> I'd prefer a mailing-list instead, once finished all the exercises, I'd 
> like to looking for somebody that he has my same handbook and to ask him 
> for exchange the exercises for comparison purpose.
> Does anybody know a mailing-list for C language questions?

  comp.lang.c which is not a mailing list but instead a
usenet group (which is much better than a mailing list
for such types of conversations).


  songbird

[toc] | [prev] | [next] | [standalone]

#275817

Franco Martelli <martellif67@gmail.com> writes:

> I'd prefer a mailing-list instead, once finished all the exercises,
> I'd like to looking for somebody that he has my same handbook and to
> ask him for exchange the exercises for comparison purpose.

Just curious, which handbook is it?

[toc] | [prev] | [next] | [standalone]

#275824

On 17/12/24 at 12:20, Anssi Saari wrote:
> Franco Martelli <martellif67@gmail.com> writes:
> 
>> I'd prefer a mailing-list instead, once finished all the exercises,
>> I'd like to looking for somebody that he has my same handbook and to
>> ask him for exchange the exercises for comparison purpose.
> 
> Just curious, which handbook is it?
> 
Peter A. Darnell, Philip E. Margolis - "C A Software Engineering Approach":

https://www.google.it/books/edition/_/1nsS5q9aZOUC?hl=it&gbpv=0

Do you have it too? It's pretty old, with some typo, but it looks to me 
good.

-- 
Franco Martelli

[toc] | [prev] | [next] | [standalone]

#275866

Franco Martelli <martellif67@gmail.com> writes:

> Peter A. Darnell, Philip E. Margolis - "C A Software Engineering Approach":
>
> https://www.google.it/books/edition/_/1nsS5q9aZOUC?hl=it&gbpv=0
>
> Do you have it too? It's pretty old, with some typo, but it looks to
> me good.

Sorry, no, doesn't look familiar. I remember I bought some C book in the
90s as a student. It had a lot of issues in the example code snippets
but that wasn't bad for learning.

[toc] | [prev] | [next] | [standalone]

#275854

Hello :)

Le 17/12/2024 à 12:20, Anssi Saari a écrit :
> Franco Martelli <martellif67@gmail.com> writes:
> 
>> I'd prefer a mailing-list instead, once finished all the exercises,
>> I'd like to looking for somebody that he has my same handbook and to
>> ask him for exchange the exercises for comparison purpose.
> 
> Just curious, which handbook is it?
Good question ! makes me curious too :)

Jeff

[toc] | [prev] | [next] | [standalone]

#275847

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.

-- 
Franco Martelli

[toc] | [prev] | [next] | [standalone]

#275850

On Tue, Dec 17, 2024 at 16:09:09 -0500, Jeffrey Walton wrote:
> I would rewrite the cleanup code like so:
> 
>     void dealloc()
>     {
>         DIGIT *next, *p = head;
>         while( p )
>             next = p->next, free( p ), p = next;
>     }

The logic looks good, but I'm not a fan of this use of the comma operator.
Is that a fashionable thing nowadays?

Especially when teaching a new programmer the ropes, I would stick with
the regular syntax.

    void dealloc() {
        DIGIT *next, *p = head;
        while (p) {
            next = p->next;
            free(p);
            p = next;
        }
    }

You can put the opening { on the next line if you prefer that way.  I
know some people have very strong feeling about that.

[toc] | [prev] | [next] | [standalone]

#275857

[Multipart message — attachments visible in raw view] — view raw

On Tue, Dec 17, 2024 at 04:18:17PM -0500, Greg Wooledge wrote:
> On Tue, Dec 17, 2024 at 16:09:09 -0500, Jeffrey Walton wrote:
> > I would rewrite the cleanup code like so:
> > 
> >     void dealloc()
> >     {
> >         DIGIT *next, *p = head;
> >         while( p )
> >             next = p->next, free( p ), p = next;
> >     }
> 
> The logic looks good, but I'm not a fan of this use of the comma operator.
> Is that a fashionable thing nowadays?
> 
> Especially when teaching a new programmer the ropes, I would stick with
> the regular syntax.
> 
>     void dealloc() {
>         DIGIT *next, *p = head;
>         while (p) {
>             next = p->next;
>             free(p);
>             p = next;
>         }
>     }
> 
> You can put the opening { on the next line if you prefer that way.  I
> know some people have very strong feeling about that.

The comma saves the braces, but then, this might be a Bad Thing, as
Apple learnt the hard way a couple of years ago [1] :-)

Now taking my tongue out-of-cheek, I'd prefer your code, too. Much
clearer and much less hidden footguns.

I'm all for concise code, but I usually revert some things in a second
pass when they seem to hurt clarity. After all, you write your code for
other people to read it.

Cheers

[1] https://dwheeler.com/essays/apple-goto-fail.html
-- 
t

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.debian.user

csiph-web