Bogus code in breakpoint.c

[Mark Kettenis <kettenis at wins dot uva dot nl>]

Can anybody believe the following code actually exists in
breakpoint.c:delete_breakpoint()?

  /* On the chance that someone will soon try again to delete this same
     bp, we mark it as deleted before freeing its storage. */
  bpt->type = bp_none;

  free ((PTR) bpt);

We should think up a nasty punishment for whoever put that comment
there.  What nasty things can one do to people with band-aid :-).  

Anyway, I'm playing with some Linux threads debugging code that
evidently tries to do what the comment suggests.  In this case
deleting a step_resume breakpoint for a thread whose infrun state was
saved and restored.  The breakpoint is deleted just fine, but a
pointer is left behind in the thread list.  Later, when the thread
list is reinitialized, thread.c:free_thread() will try to delete the
breakpoint again.  Since this may happen much later, even the band-aid
doesn't help.

The attached patch comes to mind, but I'm not at all confident that it
doesn't break any other multi-threaded targets.  Comments?

Mark

Index: thread.c
===================================================================
RCS file: /cvs/src/src/gdb/thread.c,v
retrieving revision 1.6
diff -u -p -r1.6 thread.c
--- thread.c	2000/07/30 01:48:27	1.6
+++ thread.c	2000/08/18 23:42:02
@@ -306,6 +306,7 @@ load_infrun_state (int pid, CORE_ADDR *p
   *prev_func_start = tp->prev_func_start;
   *prev_func_name = tp->prev_func_name;
   *step_resume_breakpoint = tp->step_resume_breakpoint;
+  tp->step_resume_breakpoint = NULL;
   *step_range_start = tp->step_range_start;
   *step_range_end = tp->step_range_end;
   *step_frame_address = tp->step_frame_address;