| Message ID | 20191201155220.15971-1-philippe.waroquiers@skynet.be |
|---|---|
| State | New |
| Headers | show |
On 12/1/19 3:52 PM, Philippe Waroquiers wrote: > Origin of the leaks is in prune_inferiors: prune_inferiors is first removing > the inferior to prune from the inferior list, then calls delete_inferior. > But delete_inferior will only really destroy the inferior when it finds > it into the inferior list. > As delete_inferior is removing the inferior to delete from the inferior list, > ensure prune_inferiors only calls delete_inferior, without touching the > inferior list. Whoops. > @@ -370,24 +370,22 @@ have_live_inferiors (void) > void > prune_inferiors (void) > { > - struct inferior *ss, **ss_link; > + struct inferior *ss, *ss_next; > > > - *ss_link = ss->next; > + ss_next = ss->next; > delete_inferior (ss); > - ss = *ss_link; > + ss = ss_next; Please declare ss_next here instead of at the top. You can omit the redundant "struct" while at it: inferior *next = ss->next; delete_inferior (ss); ss = next; OK with that change. Thanks! Pedro Alves
On Sun, 1 Dec 2019 16:52:20 +0100 Philippe Waroquiers <philippe.waroquiers@skynet.be> wrote: > Valgrind detects various inferior related leaks, such as: > ==31877== 5,530 (56 direct, 5,474 indirect) bytes in 1 blocks are definitely lost in loss record 7,131 of 7,355 > ==31877== at 0x4C2E18C: calloc (vg_replace_malloc.c:760) > ==31877== by 0x23E580: xcalloc (alloc.c:100) > ==31877== by 0x4794A9: xcnewvec<void*> (poison.h:158) > ==31877== by 0x4794A9: registry_alloc_data(registry_data_registry*, registry_fields*) (registry.c:51) > ==31877== by 0x3A537C: inferior_alloc_data (inferior.c:43) > ==31877== by 0x3A537C: inferior::inferior(int) (inferior.c:92) > ==31877== by 0x3A5426: add_inferior_silent(int) (inferior.c:98) > ==31877== by 0x3A5530: add_inferior(int) (inferior.c:122) > ... > > Origin of the leaks is in prune_inferiors: prune_inferiors is first removing > the inferior to prune from the inferior list, then calls delete_inferior. > But delete_inferior will only really destroy the inferior when it finds > it into the inferior list. > As delete_inferior is removing the inferior to delete from the inferior list, > ensure prune_inferiors only calls delete_inferior, without touching the > inferior list. > > gdb/ChangeLog > YYYY-MM-DD Philippe Waroquiers <philippe.waroquiers@skynet.be> > * inferior.c (prune_inferiors): Only call delete_inferior, > Do not modify the inferior list. Okay. (Though I'd rather see a semicolon or a period in place of the comma in the ChangeLog entry.) Kevin
On Thu, 2019-12-05 at 16:42 +0000, Pedro Alves wrote: > Please declare ss_next here instead of at the top. > You can omit the redundant "struct" while at it: > > inferior *next = ss->next; > delete_inferior (ss); > ss = next; > > OK with that change. Thanks! Pushed after fixing the above, and the ChangeLog message as suggested by Kevin. Thanks for the review. Philippe
diff --git a/gdb/inferior.c b/gdb/inferior.c index 84e4d24e73..55c8409cd9 100644 --- a/gdb/inferior.c +++ b/gdb/inferior.c @@ -370,24 +370,22 @@ have_live_inferiors (void) void prune_inferiors (void) { - struct inferior *ss, **ss_link; + struct inferior *ss, *ss_next; ss = inferior_list; - ss_link = &inferior_list; while (ss) { if (!ss->deletable () || !ss->removable || ss->pid != 0) { - ss_link = &ss->next; - ss = *ss_link; + ss = ss->next; continue; } - *ss_link = ss->next; + ss_next = ss->next; delete_inferior (ss); - ss = *ss_link; + ss = ss_next; } }