[patch] clean up potential memory leak in thread.c

[patch] clean up potential memory leak in thread.c

[Kris Warkentin]

[-- Attachment #1: Type: text/plain, Size: 496 bytes --]

Got rid of a 'FIXME' by implementing a target hook for cleaning up 
private thread info.  Look kosher?

cheers,

Kris

2005-09-29  Kris Warkentin  <kewarken@qnx.com>

    * target.h (struct target_ops): Add new to_delete_extra_thread_info
    hook and define target_delete_extra_thread_info macro.
    * target.c (update_current_target):  INHERIT and de_fault above target
    function.
    * thread.c (free_thread): Call target_delete_extra_thread_info before
    freeing private thread data.



[-- Attachment #2: tidinfo.diff --]
[-- Type: text/x-patch, Size: 2641 bytes --]

Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.76
diff -u -r1.76 target.h
--- target.h	4 Sep 2005 16:18:20 -0000	1.76
+++ target.h	29 Sep 2005 16:56:31 -0000
@@ -53,6 +53,7 @@
 #include "symtab.h"
 #include "dcache.h"
 #include "memattr.h"
+#include "gdbthread.h"
 
 enum strata
   {
@@ -374,6 +375,7 @@
     void (*to_find_new_threads) (void);
     char *(*to_pid_to_str) (ptid_t);
     char *(*to_extra_thread_info) (struct thread_info *);
+    void (*to_delete_extra_thread_info) (struct private_thread_info *);
     void (*to_stop) (void);
     void (*to_rcmd) (char *command, struct ui_file *output);
     struct symtab_and_line *(*to_enable_exception_callback) (enum
@@ -923,6 +925,11 @@
 #define target_extra_thread_info(TP) \
      (current_target.to_extra_thread_info (TP))
 
+/* Free any data held in private thread info.  The private_thread_info
+   structure is freed by the caller.  */
+#define target_delete_extra_thread_info(P) \
+     (current_target.to_delete_extra_thread_info (P))
+
 /*
  * New Objfile Event Hook:
  *
Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.111
diff -u -r1.111 target.c
--- target.c	4 Sep 2005 16:18:20 -0000	1.111
+++ target.c	29 Sep 2005 16:56:31 -0000
@@ -434,6 +434,7 @@
       INHERIT (to_find_new_threads, t);
       INHERIT (to_pid_to_str, t);
       INHERIT (to_extra_thread_info, t);
+      INHERIT (to_delete_extra_thread_info, t);
       INHERIT (to_stop, t);
       /* Do not inherit to_xfer_partial.  */
       INHERIT (to_rcmd, t);
@@ -608,6 +609,9 @@
   de_fault (to_extra_thread_info, 
 	    (char *(*) (struct thread_info *)) 
 	    return_zero);
+  de_fault (to_delete_extra_thread_info, 
+	    (void (*) (struct private_thread_info *)) 
+	    target_ignore);
   de_fault (to_stop, 
 	    (void (*) (void)) 
 	    target_ignore);
Index: thread.c
===================================================================
RCS file: /cvs/src/src/gdb/thread.c,v
retrieving revision 1.46
diff -u -r1.46 thread.c
--- thread.c	15 Feb 2005 15:49:22 -0000	1.46
+++ thread.c	29 Sep 2005 16:56:32 -0000
@@ -91,10 +91,11 @@
   if (tp->step_resume_breakpoint)
     delete_breakpoint (tp->step_resume_breakpoint);
 
-  /* FIXME: do I ever need to call the back-end to give it a
-     chance at this private data before deleting the thread?  */
   if (tp->private)
-    xfree (tp->private);
+    {
+      target_delete_extra_thread_info(tp->private);
+      xfree (tp->private);
+    }
 
   xfree (tp);
 }

Re: [patch] clean up potential memory leak in thread.c

[Daniel Jacobowitz]

On Thu, Sep 29, 2005 at 01:18:41PM -0400, Kris Warkentin wrote:
> Got rid of a 'FIXME' by implementing a target hook for cleaning up 
> private thread info.  Look kosher?

I assume you have some target data that you need to get rid of for QNX? 
Most other threaded targets have found a single struct sufficient, so
I'm mildly curious what you need.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [patch] clean up potential memory leak in thread.c

[Kris Warkentin]

We're throwing named thread support into the kernel so I'm keeping an 
arbitrary length string in there.  Would never be a huge memory leak but 
I'm fussy that way.

cheers,

Kris

Daniel Jacobowitz wrote:

> On Thu, Sep 29, 2005 at 01:18:41PM -0400, Kris Warkentin wrote:
> > Got rid of a 'FIXME' by implementing a target hook for cleaning up
> > private thread info.  Look kosher?
>
> I assume you have some target data that you need to get rid of for QNX?
> Most other threaded targets have found a single struct sufficient, so
> I'm mildly curious what you need.
>
> -- 
> Daniel Jacobowitz
> CodeSourcery, LLC
>

-- 
Stay up-to-date on all the QNX news!  Register at
http://www.qnx.com/news/forms/newsletter.html to
receive our newsletter.

Re: [patch] clean up potential memory leak in thread.c

[Daniel Jacobowitz]

On Mon, Oct 03, 2005 at 10:30:12AM -0400, Kris Warkentin wrote:
> We're throwing named thread support into the kernel so I'm keeping an 
> arbitrary length string in there.  Would never be a huge memory leak but 
> I'm fussy that way.

You don't actually need this to add an arbitrary lenth string; the
target gets to malloc this, you can use a trailing char[1] or simply
malloc sizeof(struct) + strlen (char) + 1 and point the char* right
after the struct.

If it's clearer to do it this way, though, we can go with your patch.
If you'd rather do that let me know; IIRC there were some formatting
glitches.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [patch] clean up potential memory leak in thread.c

[Kris Warkentin]

Actually, that's a pretty good idea.  We do that in a number of other 
places and I suppose it's no harder to realloc the whole structure than 
to just re-create the thread name string.

Consider the patch withdrawn.

cheers,

Kris

Daniel Jacobowitz wrote:

> On Mon, Oct 03, 2005 at 10:30:12AM -0400, Kris Warkentin wrote:
> > We're throwing named thread support into the kernel so I'm keeping an
> > arbitrary length string in there.  Would never be a huge memory leak 
> but
> > I'm fussy that way.
>
> You don't actually need this to add an arbitrary lenth string; the
> target gets to malloc this, you can use a trailing char[1] or simply
> malloc sizeof(struct) + strlen (char) + 1 and point the char* right
> after the struct.
>
> If it's clearer to do it this way, though, we can go with your patch.
> If you'd rather do that let me know; IIRC there were some formatting
> glitches.
>
> -- 
> Daniel Jacobowitz
> CodeSourcery, LLC
>

-- 
Stay up-to-date on all the QNX news!  Register at
http://www.qnx.com/news/forms/newsletter.html to
receive our newsletter.