Remove 'run cleanup chain'

Remove 'run cleanup chain'

[Vladimir Prus]

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


GDB now has a special 'run cleanup chain', but it seems to
be not really needed.

This cleanup chain is only used to call clear_solib in response
to 'run' command. However:
   - Just calling clear_solib is much simpler
   - We need to call clear_solib for attach, too, and it's done
   via direct call, so it's better to have run be similar to
   attach.

There's comment in jv-lang.c that hints that the run cleanup can
be useful, but is broken, and the code containing the
comment is not actually used itself.

So, it seems to me that carrying this general code for the
sake of a single client that is only complicated as result is not good,
here's a patch to remove run cleanup chain. OK?

- Volodya

[-- Attachment #2: run_cleanup.diff --]
[-- Type: text/x-diff, Size: 5076 bytes --]

commit ec9c7853b07801d6c4b0973156e5e5dfee5ead2f
Author: Vladimir Prus <vladimir@codesourcery.com>
Date:   Wed Nov 14 15:43:07 2007 +0300

    Remove 'run_cleanup'.

diff --git a/gdb/defs.h b/gdb/defs.h
index 8cfc6e3..deaa528 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -347,7 +347,6 @@ extern char *safe_strerror (int);
 
 extern void do_cleanups (struct cleanup *);
 extern void do_final_cleanups (struct cleanup *);
-extern void do_run_cleanups (struct cleanup *);
 extern void do_exec_cleanups (struct cleanup *);
 extern void do_exec_error_cleanups (struct cleanup *);
 
@@ -383,8 +382,6 @@ extern struct cleanup *make_final_cleanup (make_cleanup_ftype *, void *);
 extern struct cleanup *make_my_cleanup (struct cleanup **,
 					make_cleanup_ftype *, void *);
 
-extern struct cleanup *make_run_cleanup (make_cleanup_ftype *, void *);
-
 extern struct cleanup *make_exec_cleanup (make_cleanup_ftype *, void *);
 extern struct cleanup *make_exec_error_cleanup (make_cleanup_ftype *, void *);
 
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 079d586..e8ac46a 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -482,7 +482,7 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main)
   /* Purge old solib objfiles. */
   objfile_purge_solibs ();
 
-  do_run_cleanups (NULL);
+  clear_solib ();
 
   /* The comment here used to read, "The exec file is re-read every
      time we do a generic_mourn_inferior, so we just have to worry
diff --git a/gdb/jv-lang.c b/gdb/jv-lang.c
index a426d61..e3c6299 100644
--- a/gdb/jv-lang.c
+++ b/gdb/jv-lang.c
@@ -1133,25 +1133,3 @@ _initialize_java_language (void)
 
   add_language (&java_language_defn);
 }
-
-/* Cleanup code that should be run on every "run".
-   We should use make_run_cleanup to have this be called.
-   But will that mess up values in value histry?  FIXME */
-
-extern void java_rerun_cleanup (void);
-void
-java_rerun_cleanup (void)
-{
-  if (class_symtab != NULL)
-    {
-      free_symtab (class_symtab);	/* ??? */
-      class_symtab = NULL;
-    }
-  if (dynamics_objfile != NULL)
-    {
-      free_objfile (dynamics_objfile);
-      dynamics_objfile = NULL;
-    }
-
-  java_object_type = NULL;
-}
diff --git a/gdb/solib.c b/gdb/solib.c
index f687558..0ec0a51 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -86,12 +86,8 @@ struct target_so_ops *current_target_so_ops;
 
 static struct so_list *so_list_head;	/* List of known shared objects */
 
-static int solib_cleanup_queued = 0;	/* make_run_cleanup called */
-
 /* Local function prototypes */
 
-static void do_clear_solib (void *);
-
 /* If non-empty, this is a search path for loading non-absolute shared library
    symbol files.  This takes precedence over the environment variables PATH
    and LD_LIBRARY_PATH.  */
@@ -506,15 +502,6 @@ update_solib_list (int from_tty, struct target_ops *target)
 		  "Error reading attached process's symbol file.\n",
 		  RETURN_MASK_ALL);
 
-  /* Since this function might actually add some elements to the
-     so_list_head list, arrange for it to be cleaned up when
-     appropriate.  */
-  if (!solib_cleanup_queued)
-    {
-      make_run_cleanup (do_clear_solib, NULL);
-      solib_cleanup_queued = 1;
-    }
-
   /* GDB and the inferior's dynamic linker each maintain their own
      list of currently loaded shared objects; we want to bring the
      former in sync with the latter.  Scan both lists, seeing which
@@ -866,13 +853,6 @@ clear_solib (void)
   ops->clear_solib ();
 }
 
-static void
-do_clear_solib (void *dummy)
-{
-  solib_cleanup_queued = 0;
-  clear_solib ();
-}
-
 /* GLOBAL FUNCTION
 
    solib_create_inferior_hook -- shared library startup support
@@ -955,7 +935,7 @@ void
 no_shared_libraries (char *ignored, int from_tty)
 {
   objfile_purge_solibs ();
-  do_clear_solib (NULL);
+  clear_solib ();
 }
 
 static void
diff --git a/gdb/utils.c b/gdb/utils.c
index af2fac8..3be084e 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -96,7 +96,6 @@ static void set_width (void);
 
 static struct cleanup *cleanup_chain;	/* cleaned up after a failed command */
 static struct cleanup *final_cleanup_chain;	/* cleaned up when gdb exits */
-static struct cleanup *run_cleanup_chain;	/* cleaned up on each 'run' */
 static struct cleanup *exec_cleanup_chain;	/* cleaned up on each execution command */
 /* cleaned up on each error from within an execution command */
 static struct cleanup *exec_error_cleanup_chain;
@@ -210,12 +209,6 @@ make_final_cleanup (make_cleanup_ftype *function, void *arg)
 }
 
 struct cleanup *
-make_run_cleanup (make_cleanup_ftype *function, void *arg)
-{
-  return make_my_cleanup (&run_cleanup_chain, function, arg);
-}
-
-struct cleanup *
 make_exec_cleanup (make_cleanup_ftype *function, void *arg)
 {
   return make_my_cleanup (&exec_cleanup_chain, function, arg);
@@ -324,12 +317,6 @@ do_final_cleanups (struct cleanup *old_chain)
 }
 
 void
-do_run_cleanups (struct cleanup *old_chain)
-{
-  do_my_cleanups (&run_cleanup_chain, old_chain);
-}
-
-void
 do_exec_cleanups (struct cleanup *old_chain)
 {
   do_my_cleanups (&exec_cleanup_chain, old_chain);

Re: Remove 'run cleanup chain'

[Jim Blandy]

Vladimir Prus <vladimir at codesourcery.com> writes:
> GDB now has a special 'run cleanup chain', but it seems to
> be not really needed.
>
> This cleanup chain is only used to call clear_solib in response
> to 'run' command. However:
>    - Just calling clear_solib is much simpler
>    - We need to call clear_solib for attach, too, and it's done
>    via direct call, so it's better to have run be similar to
>    attach.
>
> There's comment in jv-lang.c that hints that the run cleanup can
> be useful, but is broken, and the code containing the
> comment is not actually used itself.
>
> So, it seems to me that carrying this general code for the
> sake of a single client that is only complicated as result is not good,
> here's a patch to remove run cleanup chain. OK?

This needs a ChangeLog entry.  :)

Re: Remove 'run cleanup chain'

[Vladimir Prus]

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

Jim Blandy wrote:

> 
> Vladimir Prus <vladimir at codesourcery.com> writes:
>> GDB now has a special 'run cleanup chain', but it seems to
>> be not really needed.
>>
>> This cleanup chain is only used to call clear_solib in response
>> to 'run' command. However:
>>    - Just calling clear_solib is much simpler
>>    - We need to call clear_solib for attach, too, and it's done
>>    via direct call, so it's better to have run be similar to
>>    attach.
>>
>> There's comment in jv-lang.c that hints that the run cleanup can
>> be useful, but is broken, and the code containing the
>> comment is not actually used itself.
>>
>> So, it seems to me that carrying this general code for the
>> sake of a single client that is only complicated as result is not good,
>> here's a patch to remove run cleanup chain. OK?
> 
> This needs a ChangeLog entry.  :)

Thanks, the patch with ChangeLog entry that I've checked in is attached.

- Volodya




[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: run_cleanup_final.diff --]
[-- Type: text/x-diff; name="run_cleanup_final.diff", Size: 6838 bytes --]

? .git
? 0001-Remove-CLEAR_SOLIB-use.patch
? 0002-Remove-run_cleanup.patch
? 0003-Prevent-clear_command-from-directly-modifying-breakp.patch
? A.diff
? clear_solib_final.diff
? run_cleanup_final.diff
Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/ChangeLog,v
retrieving revision 1.770
diff -u -p -r1.770 ChangeLog
--- ChangeLog	15 Nov 2007 06:14:25 -0000	1.770
+++ ChangeLog	15 Nov 2007 06:23:16 -0000
@@ -1,5 +1,20 @@
 2007-10-15  Vladimir Prus  <vladimir@codesourcery.com>
 
+	Remove 'run_cleanup'.
+	* defs.h (do_run_cleanups, make_run_cleanup): Remove
+	declarations.
+	* infcmd.c (run_command_1): Call clear_solib instead
+	of do_run_cleanups.
+	* jv-lang.c (java_rerun_cleanup): Remove, for lack of
+	any use.
+	* solib.c (solib_cleanup_queued, do_clear_solib): Remove.
+	(update_solib_lib): Don't setup run cleanup.
+	(no_shared_libraries): Call clear_solib, not do_clear_solib.
+	* utils.c (run_cleanup_chain, make_run_cleanup)
+	(do_run_cleanups): Remove.
+		
+2007-10-15  Vladimir Prus  <vladimir@codesourcery.com>
+
 	Remove CLEAR_SOLIB use.
 	* corelow.c (core_close): Don't check for CLEAR_SOLIB.
 	* infcmd.c (attach_command): Likewise.
Index: gdb/defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.210
diff -u -p -r1.210 defs.h
--- gdb/defs.h	13 Oct 2007 00:06:52 -0000	1.210
+++ gdb/defs.h	15 Nov 2007 06:23:17 -0000
@@ -347,7 +347,6 @@ extern char *safe_strerror (int);
 
 extern void do_cleanups (struct cleanup *);
 extern void do_final_cleanups (struct cleanup *);
-extern void do_run_cleanups (struct cleanup *);
 extern void do_exec_cleanups (struct cleanup *);
 extern void do_exec_error_cleanups (struct cleanup *);
 
@@ -383,8 +382,6 @@ extern struct cleanup *make_final_cleanu
 extern struct cleanup *make_my_cleanup (struct cleanup **,
 					make_cleanup_ftype *, void *);
 
-extern struct cleanup *make_run_cleanup (make_cleanup_ftype *, void *);
-
 extern struct cleanup *make_exec_cleanup (make_cleanup_ftype *, void *);
 extern struct cleanup *make_exec_error_cleanup (make_cleanup_ftype *, void *);
 
Index: gdb/infcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/infcmd.c,v
retrieving revision 1.163
diff -u -p -r1.163 infcmd.c
--- gdb/infcmd.c	15 Nov 2007 06:14:26 -0000	1.163
+++ gdb/infcmd.c	15 Nov 2007 06:23:17 -0000
@@ -482,7 +482,7 @@ run_command_1 (char *args, int from_tty,
   /* Purge old solib objfiles. */
   objfile_purge_solibs ();
 
-  do_run_cleanups (NULL);
+  clear_solib ();
 
   /* The comment here used to read, "The exec file is re-read every
      time we do a generic_mourn_inferior, so we just have to worry
Index: gdb/jv-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/jv-lang.c,v
retrieving revision 1.51
diff -u -p -r1.51 jv-lang.c
--- gdb/jv-lang.c	2 Nov 2007 19:34:11 -0000	1.51
+++ gdb/jv-lang.c	15 Nov 2007 06:23:18 -0000
@@ -1133,25 +1133,3 @@ _initialize_java_language (void)
 
   add_language (&java_language_defn);
 }
-
-/* Cleanup code that should be run on every "run".
-   We should use make_run_cleanup to have this be called.
-   But will that mess up values in value histry?  FIXME */
-
-extern void java_rerun_cleanup (void);
-void
-java_rerun_cleanup (void)
-{
-  if (class_symtab != NULL)
-    {
-      free_symtab (class_symtab);	/* ??? */
-      class_symtab = NULL;
-    }
-  if (dynamics_objfile != NULL)
-    {
-      free_objfile (dynamics_objfile);
-      dynamics_objfile = NULL;
-    }
-
-  java_object_type = NULL;
-}
Index: gdb/solib.c
===================================================================
RCS file: /cvs/src/src/gdb/solib.c,v
retrieving revision 1.98
diff -u -p -r1.98 solib.c
--- gdb/solib.c	24 Oct 2007 21:07:30 -0000	1.98
+++ gdb/solib.c	15 Nov 2007 06:23:19 -0000
@@ -86,12 +86,8 @@ struct target_so_ops *current_target_so_
 
 static struct so_list *so_list_head;	/* List of known shared objects */
 
-static int solib_cleanup_queued = 0;	/* make_run_cleanup called */
-
 /* Local function prototypes */
 
-static void do_clear_solib (void *);
-
 /* If non-empty, this is a search path for loading non-absolute shared library
    symbol files.  This takes precedence over the environment variables PATH
    and LD_LIBRARY_PATH.  */
@@ -506,15 +502,6 @@ update_solib_list (int from_tty, struct 
 		  "Error reading attached process's symbol file.\n",
 		  RETURN_MASK_ALL);
 
-  /* Since this function might actually add some elements to the
-     so_list_head list, arrange for it to be cleaned up when
-     appropriate.  */
-  if (!solib_cleanup_queued)
-    {
-      make_run_cleanup (do_clear_solib, NULL);
-      solib_cleanup_queued = 1;
-    }
-
   /* GDB and the inferior's dynamic linker each maintain their own
      list of currently loaded shared objects; we want to bring the
      former in sync with the latter.  Scan both lists, seeing which
@@ -866,13 +853,6 @@ clear_solib (void)
   ops->clear_solib ();
 }
 
-static void
-do_clear_solib (void *dummy)
-{
-  solib_cleanup_queued = 0;
-  clear_solib ();
-}
-
 /* GLOBAL FUNCTION
 
    solib_create_inferior_hook -- shared library startup support
@@ -955,7 +935,7 @@ void
 no_shared_libraries (char *ignored, int from_tty)
 {
   objfile_purge_solibs ();
-  do_clear_solib (NULL);
+  clear_solib ();
 }
 
 static void
Index: gdb/utils.c
===================================================================
RCS file: /cvs/src/src/gdb/utils.c,v
retrieving revision 1.181
diff -u -p -r1.181 utils.c
--- gdb/utils.c	23 Aug 2007 18:08:46 -0000	1.181
+++ gdb/utils.c	15 Nov 2007 06:23:19 -0000
@@ -96,7 +96,6 @@ static void set_width (void);
 
 static struct cleanup *cleanup_chain;	/* cleaned up after a failed command */
 static struct cleanup *final_cleanup_chain;	/* cleaned up when gdb exits */
-static struct cleanup *run_cleanup_chain;	/* cleaned up on each 'run' */
 static struct cleanup *exec_cleanup_chain;	/* cleaned up on each execution command */
 /* cleaned up on each error from within an execution command */
 static struct cleanup *exec_error_cleanup_chain;
@@ -210,12 +209,6 @@ make_final_cleanup (make_cleanup_ftype *
 }
 
 struct cleanup *
-make_run_cleanup (make_cleanup_ftype *function, void *arg)
-{
-  return make_my_cleanup (&run_cleanup_chain, function, arg);
-}
-
-struct cleanup *
 make_exec_cleanup (make_cleanup_ftype *function, void *arg)
 {
   return make_my_cleanup (&exec_cleanup_chain, function, arg);
@@ -324,12 +317,6 @@ do_final_cleanups (struct cleanup *old_c
 }
 
 void
-do_run_cleanups (struct cleanup *old_chain)
-{
-  do_my_cleanups (&run_cleanup_chain, old_chain);
-}
-
-void
 do_exec_cleanups (struct cleanup *old_chain)
 {
   do_my_cleanups (&exec_cleanup_chain, old_chain);

Re: Remove 'run cleanup chain'

[Markus Deuling]

Hi Vladimir,

Vladimir Prus schrieb:
> Thanks, the patch with ChangeLog entry that I've checked in is attached.

I think you got the wrong ChangeLog file (the one in src/ instead of gdb/) ?


Regards,
Markus

-- 
Markus Deuling
GNU Toolchain for Linux on Cell BE
deuling@de.ibm.com

Re: Remove 'run cleanup chain'

[Vladimir Prus]

On Thursday 15 November 2007 10:03:27 you wrote:
> Hi Vladimir,
> 
> Vladimir Prus schrieb:
> > Thanks, the patch with ChangeLog entry that I've checked in is attached.
> 
> I think you got the wrong ChangeLog file (the one in src/ instead of gdb/) ?

Doh! As it turns out I also don't know what is the number of the current month.

I've moved the entries to the right file.

Sorry,
Volodya