Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] Make map_symbol_filenames_psymtab interruptible
@ 2011-10-27 20:29 Sterling Augustine
  2011-10-28 15:59 ` Tom Tromey
  0 siblings, 1 reply; 5+ messages in thread
From: Sterling Augustine @ 2011-10-27 20:29 UTC (permalink / raw)
  To: gdb-patches

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

Hi,

Enclosed is a patch to allow interrupting
map_symbol_filenames_psymtab. This patch adds a call to QUIT in
map_symbol_filenames_psymtab, and several functions to cleanup data
that would have leaked.

Also, this patch adds a cleanup for the existing leak in
default_make_symbol_completion_list_break_on (check return_val's
relation to the existing QUITs). Although the new QUIT doesn't
introduce this particular leak, it creates many more opportunities for
it to happen.

Thanks,

Sterling
--
2011-10-27  Sterling Augustine  <saugustine@google.com>

	* psymtab.c (map_symbol_filenames_psymtab): Call QUIT.
	* symtab.c (free_completion_list): New function.
	(do_free_completion_list): Likewise.
	(default_make_symbol_completion_list_break_on): New variable
	back_to. Call make_cleanup and discard_cleanups.
	(make_source_files_completion_list): Likewise.

[-- Attachment #2: cleanup.patch --]
[-- Type: text/x-patch, Size: 2478 bytes --]

Index: psymtab.c
===================================================================
RCS file: /cvs/src/src/gdb/psymtab.c,v
retrieving revision 1.30
diff -d -u -r1.30 psymtab.c
--- psymtab.c	10 Jun 2011 21:48:04 -0000	1.30
+++ psymtab.c	27 Oct 2011 19:39:02 -0000
@@ -1093,6 +1093,7 @@
       if (ps->readin)
 	continue;
 
+      QUIT;
       fullname = psymtab_to_fullname (ps);
       (*fun) (ps->filename, fullname, data);
     }
Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.283
diff -d -u -r1.283 symtab.c
--- symtab.c	21 Jul 2011 15:13:29 -0000	1.283
+++ symtab.c	27 Oct 2011 19:39:02 -0000
@@ -3573,6 +3573,26 @@
   return 1;
 }
 
+/* Free any memory associated with a completion list.  */
+static void
+free_completion_list (char ***list_ptr)
+{
+  int i = 0;
+  char **list = *list_ptr;
+  while (list[i] != NULL)
+    {
+      xfree (list[i]);
+      i++;
+    }
+  xfree (list);
+}
+
+static void
+do_free_completion_list (void *list)
+{
+  free_completion_list (list);
+}
+
 /* Helper routine for make_symbol_completion_list.  */
 
 static int return_val_size;
@@ -3810,6 +3830,7 @@
   /* Length of sym_text.  */
   int sym_text_len;
   struct add_name_data datum;
+  struct cleanup *back_to;
 
   /* Now look for the symbol we are supposed to complete on.  */
   {
@@ -3886,6 +3907,7 @@
   return_val_index = 0;
   return_val = (char **) xmalloc ((return_val_size + 1) * sizeof (char *));
   return_val[0] = NULL;
+  back_to = make_cleanup (do_free_completion_list, &return_val);
 
   datum.sym_text = sym_text;
   datum.sym_text_len = sym_text_len;
@@ -3995,6 +4017,7 @@
       macro_for_each (macro_user_macros, add_macro_name, &datum);
     }
 
+  discard_cleanups (back_to);
   return (return_val);
 }
 
@@ -4244,12 +4267,15 @@
   char **list = (char **) xmalloc (list_alloced * sizeof (char *));
   const char *base_name;
   struct add_partial_filename_data datum;
+  struct cleanup *back_to;
 
   list[0] = NULL;
 
   if (!have_full_symbols () && !have_partial_symbols ())
     return list;
 
+  back_to = make_cleanup (do_free_completion_list, &list);
+
   ALL_SYMTABS (objfile, s)
     {
       if (not_interesting_fname (s->filename))
@@ -4285,6 +4311,7 @@
   datum.list_used = &list_used;
   datum.list_alloced = &list_alloced;
   map_partial_symbol_filenames (maybe_add_partial_symtab_filename, &datum);
+  discard_cleanups (back_to);
 
   return list;
 }

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Make map_symbol_filenames_psymtab interruptible
  2011-10-27 20:29 [PATCH] Make map_symbol_filenames_psymtab interruptible Sterling Augustine
@ 2011-10-28 15:59 ` Tom Tromey
  2011-10-28 17:36   ` Sterling Augustine
  0 siblings, 1 reply; 5+ messages in thread
From: Tom Tromey @ 2011-10-28 15:59 UTC (permalink / raw)
  To: Sterling Augustine; +Cc: gdb-patches

>>>>> "Sterling" == Sterling Augustine <saugustine@google.com> writes:

Sterling> Enclosed is a patch to allow interrupting
Sterling> map_symbol_filenames_psymtab. This patch adds a call to QUIT in
Sterling> map_symbol_filenames_psymtab, and several functions to cleanup data
Sterling> that would have leaked.

Thanks.

Sterling> +/* Free any memory associated with a completion list.  */
Sterling> +static void

Newline after comment.

Sterling> +free_completion_list (char ***list_ptr)
Sterling> +{
Sterling> +  int i = 0;
Sterling> +  char **list = *list_ptr;
Sterling> +  while (list[i] != NULL)

Newline after declaration block.

Sterling> +
Sterling> +static void
Sterling> +do_free_completion_list (void *list)

Comment before this function.

Ok with those changes.

Tom


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Make map_symbol_filenames_psymtab interruptible
  2011-10-28 15:59 ` Tom Tromey
@ 2011-10-28 17:36   ` Sterling Augustine
  2011-10-28 18:14     ` Sterling Augustine
  0 siblings, 1 reply; 5+ messages in thread
From: Sterling Augustine @ 2011-10-28 17:36 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On Fri, Oct 28, 2011 at 7:55 AM, Tom Tromey <tromey@redhat.com> wrote:
> Ok with those changes.

Checked in with those fixes.

Thanks.

Sterling


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Make map_symbol_filenames_psymtab interruptible
  2011-10-28 17:36   ` Sterling Augustine
@ 2011-10-28 18:14     ` Sterling Augustine
  2011-11-02 20:48       ` Phil Muldoon
  0 siblings, 1 reply; 5+ messages in thread
From: Sterling Augustine @ 2011-10-28 18:14 UTC (permalink / raw)
  To: gdb-patches

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

And here is the exact patch....

Sterling

[-- Attachment #2: cleanup.patch --]
[-- Type: text/x-patch, Size: 9680 bytes --]

cvs diff: Diffing .
Index: psymtab.c
===================================================================
RCS file: /cvs/src/src/gdb/psymtab.c,v
retrieving revision 1.30
diff -u -r1.30 psymtab.c
--- psymtab.c	10 Jun 2011 21:48:04 -0000	1.30
+++ psymtab.c	28 Oct 2011 17:27:15 -0000
@@ -1093,6 +1093,7 @@
       if (ps->readin)
 	continue;
 
+      QUIT;
       fullname = psymtab_to_fullname (ps);
       (*fun) (ps->filename, fullname, data);
     }
Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.283
diff -u -r1.283 symtab.c
--- symtab.c	21 Jul 2011 15:13:29 -0000	1.283
+++ symtab.c	28 Oct 2011 17:27:15 -0000
@@ -3573,6 +3573,30 @@
   return 1;
 }
 
+/* Free any memory associated with a completion list.  */
+
+static void
+free_completion_list (char ***list_ptr)
+{
+  int i = 0;
+  char **list = *list_ptr;
+
+  while (list[i] != NULL)
+    {
+      xfree (list[i]);
+      i++;
+    }
+  xfree (list);
+}
+
+/* Callback for make_cleanup.  */
+
+static void
+do_free_completion_list (void *list)
+{
+  free_completion_list (list);
+}
+
 /* Helper routine for make_symbol_completion_list.  */
 
 static int return_val_size;
@@ -3810,6 +3834,7 @@
   /* Length of sym_text.  */
   int sym_text_len;
   struct add_name_data datum;
+  struct cleanup *back_to;
 
   /* Now look for the symbol we are supposed to complete on.  */
   {
@@ -3886,6 +3911,7 @@
   return_val_index = 0;
   return_val = (char **) xmalloc ((return_val_size + 1) * sizeof (char *));
   return_val[0] = NULL;
+  back_to = make_cleanup (do_free_completion_list, &return_val);
 
   datum.sym_text = sym_text;
   datum.sym_text_len = sym_text_len;
@@ -3995,6 +4021,7 @@
       macro_for_each (macro_user_macros, add_macro_name, &datum);
     }
 
+  discard_cleanups (back_to);
   return (return_val);
 }
 
@@ -4244,12 +4271,15 @@
   char **list = (char **) xmalloc (list_alloced * sizeof (char *));
   const char *base_name;
   struct add_partial_filename_data datum;
+  struct cleanup *back_to;
 
   list[0] = NULL;
 
   if (!have_full_symbols () && !have_partial_symbols ())
     return list;
 
+  back_to = make_cleanup (do_free_completion_list, &list);
+
   ALL_SYMTABS (objfile, s)
     {
       if (not_interesting_fname (s->filename))
@@ -4285,6 +4315,7 @@
   datum.list_used = &list_used;
   datum.list_alloced = &list_alloced;
   map_partial_symbol_filenames (maybe_add_partial_symtab_filename, &datum);
+  discard_cleanups (back_to);
 
   return list;
 }
cvs diff: Diffing 29k-share
cvs diff: Diffing 29k-share/udi
cvs diff: Diffing cli
cvs diff: Diffing common
cvs diff: Diffing config
cvs diff: Diffing config/a29k
cvs diff: Diffing config/alpha
cvs diff: Diffing config/arc
cvs diff: Diffing config/arm
cvs diff: Diffing config/avr
cvs diff: Diffing config/convex
cvs diff: Diffing config/cris
cvs diff: Diffing config/d10v
cvs diff: Diffing config/d30v
cvs diff: Diffing config/djgpp
cvs diff: Diffing config/fr30
cvs diff: Diffing config/frv
cvs diff: Diffing config/gould
cvs diff: Diffing config/h8300
cvs diff: Diffing config/h8500
cvs diff: Diffing config/i386
cvs diff: Diffing config/i960
cvs diff: Diffing config/ia64
cvs diff: Diffing config/iq2000
cvs diff: Diffing config/m32c
cvs diff: Diffing config/m32r
cvs diff: Diffing config/m68hc11
cvs diff: Diffing config/m68k
cvs diff: Diffing config/m88k
cvs diff: Diffing config/mcore
cvs diff: Diffing config/mep
cvs diff: Diffing config/mips
cvs diff: Diffing config/mn10200
cvs diff: Diffing config/mn10300
cvs diff: Diffing config/ms1
cvs diff: Diffing config/mt
cvs diff: Diffing config/none
cvs diff: Diffing config/ns32k
cvs diff: Diffing config/pa
cvs diff: Diffing config/powerpc
cvs diff: Diffing config/pyr
cvs diff: Diffing config/romp
cvs diff: Diffing config/rs6000
cvs diff: Diffing config/s390
cvs diff: Diffing config/score
cvs diff: Diffing config/sh
cvs diff: Diffing config/sh64
cvs diff: Diffing config/sparc
cvs diff: Diffing config/spu
cvs diff: Diffing config/tahoe
cvs diff: Diffing config/tic80
cvs diff: Diffing config/v850
cvs diff: Diffing config/vax
cvs diff: Diffing config/w65
cvs diff: Diffing config/xstormy16
cvs diff: Diffing config/xtensa
cvs diff: Diffing config/z8k
cvs diff: Diffing data-directory
cvs diff: Diffing doc
cvs diff: Diffing features
cvs diff: Diffing features/i386
cvs diff: Diffing features/rs6000
cvs diff: Diffing gdbserver
cvs diff: Diffing gnulib
cvs diff: Diffing gnulib/extra
cvs diff: Diffing gnulib/m4
cvs diff: Diffing mi
cvs diff: Diffing nindy-share
cvs diff: Diffing nlm
cvs diff: Diffing osf-share
cvs diff: Diffing osf-share/AT386
cvs diff: Diffing osf-share/HP800
cvs diff: Diffing osf-share/RIOS
cvs diff: Diffing po
cvs diff: Diffing python
cvs diff: Diffing python/lib
cvs diff: Diffing python/lib/gdb
cvs diff: Diffing python/lib/gdb/command
cvs diff: Diffing rdi-share
cvs diff: Diffing regformats
cvs diff: Diffing regformats/i386
cvs diff: Diffing regformats/rs6000
cvs diff: Diffing signals
cvs diff: Diffing syscalls
cvs diff: Diffing testsuite
cvs diff: Diffing testsuite/config
cvs diff: Diffing testsuite/gdb.ada
cvs diff: Diffing testsuite/gdb.ada/array_bounds
cvs diff: Diffing testsuite/gdb.ada/array_first_last
cvs diff: Diffing testsuite/gdb.ada/array_return
cvs diff: Diffing testsuite/gdb.ada/array_subscript_addr
cvs diff: Diffing testsuite/gdb.ada/arrayidx
cvs diff: Diffing testsuite/gdb.ada/arrayparam
cvs diff: Diffing testsuite/gdb.ada/arrayptr
cvs diff: Diffing testsuite/gdb.ada/atomic_enum
cvs diff: Diffing testsuite/gdb.ada/call_pn
cvs diff: Diffing testsuite/gdb.ada/catch_ex
cvs diff: Diffing testsuite/gdb.ada/char_enum
cvs diff: Diffing testsuite/gdb.ada/char_param
cvs diff: Diffing testsuite/gdb.ada/complete
cvs diff: Diffing testsuite/gdb.ada/cond_lang
cvs diff: Diffing testsuite/gdb.ada/dyn_loc
cvs diff: Diffing testsuite/gdb.ada/exec_changed
cvs diff: Diffing testsuite/gdb.ada/exprs
cvs diff: Diffing testsuite/gdb.ada/fixed_cmp
cvs diff: Diffing testsuite/gdb.ada/fixed_points
cvs diff: Diffing testsuite/gdb.ada/formatted_ref
cvs diff: Diffing testsuite/gdb.ada/frame_args
cvs diff: Diffing testsuite/gdb.ada/fun_addr
cvs diff: Diffing testsuite/gdb.ada/fun_in_declare
cvs diff: Diffing testsuite/gdb.ada/funcall_param
cvs diff: Diffing testsuite/gdb.ada/homonym
cvs diff: Diffing testsuite/gdb.ada/int_deref
cvs diff: Diffing testsuite/gdb.ada/interface
cvs diff: Diffing testsuite/gdb.ada/lang_switch
cvs diff: Diffing testsuite/gdb.ada/mi_catch_ex
cvs diff: Diffing testsuite/gdb.ada/mod_from_name
cvs diff: Diffing testsuite/gdb.ada/nested
cvs diff: Diffing testsuite/gdb.ada/null_array
cvs diff: Diffing testsuite/gdb.ada/null_record
cvs diff: Diffing testsuite/gdb.ada/packed_array
cvs diff: Diffing testsuite/gdb.ada/packed_tagged
cvs diff: Diffing testsuite/gdb.ada/print_chars
cvs diff: Diffing testsuite/gdb.ada/ptr_typedef
cvs diff: Diffing testsuite/gdb.ada/ptype_field
cvs diff: Diffing testsuite/gdb.ada/ptype_tagged_param
cvs diff: Diffing testsuite/gdb.ada/rec_return
cvs diff: Diffing testsuite/gdb.ada/ref_param
cvs diff: Diffing testsuite/gdb.ada/ref_tick_size
cvs diff: Diffing testsuite/gdb.ada/same_enum
cvs diff: Diffing testsuite/gdb.ada/start
cvs diff: Diffing testsuite/gdb.ada/str_ref_cmp
cvs diff: Diffing testsuite/gdb.ada/sym_print_name
cvs diff: Diffing testsuite/gdb.ada/taft_type
cvs diff: Diffing testsuite/gdb.ada/tagged
cvs diff: Diffing testsuite/gdb.ada/tasks
cvs diff: Diffing testsuite/gdb.ada/tick_last_segv
cvs diff: Diffing testsuite/gdb.ada/type_coercion
cvs diff: Diffing testsuite/gdb.ada/uninitialized_vars
cvs diff: Diffing testsuite/gdb.ada/variant_record_packed_array
cvs diff: Diffing testsuite/gdb.ada/watch_arg
cvs diff: Diffing testsuite/gdb.ada/widewide
cvs diff: Diffing testsuite/gdb.arch
cvs diff: Diffing testsuite/gdb.asm
cvs diff: Diffing testsuite/gdb.base
cvs diff: Diffing testsuite/gdb.base/comp-dir
cvs diff: Diffing testsuite/gdb.base/comp-dir/subdir
cvs diff: Diffing testsuite/gdb.c++
cvs diff: Diffing testsuite/gdb.cell
cvs diff: Diffing testsuite/gdb.chill
cvs diff: Diffing testsuite/gdb.cp
cvs diff: Diffing testsuite/gdb.disasm
cvs diff: Diffing testsuite/gdb.dwarf2
cvs diff: Diffing testsuite/gdb.fortran
cvs diff: Diffing testsuite/gdb.gdb
cvs diff: Diffing testsuite/gdb.gdbserver
cvs diff: Diffing testsuite/gdb.hp
cvs diff: Diffing testsuite/gdb.hp/gdb.aCC
cvs diff: Diffing testsuite/gdb.hp/gdb.base-hp
cvs diff: Diffing testsuite/gdb.hp/gdb.compat
cvs diff: Diffing testsuite/gdb.hp/gdb.defects
cvs diff: Diffing testsuite/gdb.hp/gdb.objdbg
cvs diff: Diffing testsuite/gdb.hp/gdb.objdbg/objdbg01
cvs diff: Diffing testsuite/gdb.hp/gdb.objdbg/objdbg02
cvs diff: Diffing testsuite/gdb.hp/gdb.objdbg/objdbg03
cvs diff: Diffing testsuite/gdb.hp/gdb.objdbg/objdbg04
cvs diff: Diffing testsuite/gdb.hp/gdb.objdbg/tools
cvs diff: Diffing testsuite/gdb.hp/gdb.threads-hp
cvs diff: Diffing testsuite/gdb.hp/tools
cvs diff: Diffing testsuite/gdb.java
cvs diff: Diffing testsuite/gdb.mi
cvs diff: Diffing testsuite/gdb.modula2
cvs diff: Diffing testsuite/gdb.multi
cvs diff: Diffing testsuite/gdb.objc
cvs diff: Diffing testsuite/gdb.opencl
cvs diff: Diffing testsuite/gdb.opt
cvs diff: Diffing testsuite/gdb.pascal
cvs diff: Diffing testsuite/gdb.pascal/hello
cvs diff: Diffing testsuite/gdb.python
cvs diff: Diffing testsuite/gdb.reverse
cvs diff: Diffing testsuite/gdb.server
cvs diff: Diffing testsuite/gdb.stabs
cvs diff: Diffing testsuite/gdb.threads
cvs diff: Diffing testsuite/gdb.trace
cvs diff: Diffing testsuite/gdb.twreverse
cvs diff: Diffing testsuite/gdb.xml
cvs diff: Diffing testsuite/lib
cvs diff: Diffing tui
cvs diff: Diffing vx-share

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Make map_symbol_filenames_psymtab interruptible
  2011-10-28 18:14     ` Sterling Augustine
@ 2011-11-02 20:48       ` Phil Muldoon
  0 siblings, 0 replies; 5+ messages in thread
From: Phil Muldoon @ 2011-11-02 20:48 UTC (permalink / raw)
  To: Sterling Augustine; +Cc: gdb-patches

Sterling Augustine <saugustine@google.com> writes:

> And here is the exact patch....

Just a handy tip ...

> cvs diff: Diffing 29k-share
> cvs diff: Diffing 29k-share/udi
> cvs diff: Diffing cli
> cvs diff: Diffing common
> cvs diff: Diffing config
> cvs diff: Diffing config/a29k
> cvs diff: Diffing config/alpha

...

You can make CVS less annoying and behave less verbally with:

cvs -nq 

so

cvs -nq diff -u

Well, until you join the GIT side of the pool ;)

Cheers,

Phil


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2011-11-02 20:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-27 20:29 [PATCH] Make map_symbol_filenames_psymtab interruptible Sterling Augustine
2011-10-28 15:59 ` Tom Tromey
2011-10-28 17:36   ` Sterling Augustine
2011-10-28 18:14     ` Sterling Augustine
2011-11-02 20:48       ` Phil Muldoon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox