Re: [patch] Remove one immediate_quit++-- (in auto-load.c)

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> Tom has notified me on #gdb there was one needless and probably
Jan> even incorrect immediate_quit++ (and --).

I audited all the modifications of immediate_quit.  Many of them are
bogus.

immediate_quit is hard to use reliably.  I think it should only really
be done around I/O.

I'm checking in this patch.  It either removes uses of immediate_quit or
replaces them with QUIT.


Note that there are still some bad uses:

* remote_start_remote increments it but:

  1. Does a non-trivial amount of work before decrementing it.
     It is hard to believe that all this work is really
     immediate-quit-safe.

  2. AFAICT, never decrements it along the non-stop path.
     This seems like it could cause many kinds of bugs.

  3. Calls 'error' with immediate_quit still incremented.

* command_line_input and prompt_for_continue both do quite a bit of work
  with immediate_quit non-zero.  I'm not confident about the results.

I didn't try touching these.

Tom

2012-07-26  Tom Tromey  <tromey@redhat.com>

	* symmisc.c (print_symbol_bcache_statistics): Use QUIT, not
	immediate_quit.
	(print_objfile_statistics): Likewise.
	(maintenance_print_symbols): Likewise.
	(maintenance_print_msymbols): Likewise.
	(maintenance_print_objfiles): Likewise.
	* psymtab.c (print_partial_symbols): Call QUIT.
	(maintenance_print_psymbols): Likewise.  Don't modify
	immediate_quit.
	* copying.c (show_copying_command): Don't modify immediate_quit.
	(show_warranty_command): Likewise.
	* cli/cli-cmds.c (show_version): Don't modify immediate_quit.

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 24d55c3..f2328e8 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -319,10 +319,8 @@ is_complete_command (struct cmd_list_element *c)
 static void
 show_version (char *args, int from_tty)
 {
-  immediate_quit++;
   print_gdb_version (gdb_stdout);
   printf_filtered ("\n");
-  immediate_quit--;
 }
 
 /* Handle the quit command.  */
diff --git a/gdb/copying.c b/gdb/copying.c
index d608a63..9aac2e6 100644
--- a/gdb/copying.c
+++ b/gdb/copying.c
@@ -11,15 +11,13 @@ static void show_warranty_command (char *, int);
 
 void _initialize_copying (void);
 
-extern int immediate_quit;
 static void
 show_copying_command (char *ignore, int from_tty)
 {
-  immediate_quit++;
   printf_filtered ("                    GNU GENERAL PUBLIC LICENSE\n");
   printf_filtered ("                       Version 3, 29 June 2007\n");
   printf_filtered ("\n");
-  printf_filtered (" Copyright (C) 2007 Free Software Foundation, Inc. <http://fsf.org/>\n");
+  printf_filtered (" Copyright (C) 2007, 2012 Free Software Foundation, Inc. <http://fsf.org/>\n");
   printf_filtered (" Everyone is permitted to copy and distribute verbatim copies\n");
   printf_filtered (" of this license document, but changing it is not allowed.\n");
   printf_filtered ("\n");
@@ -604,13 +602,11 @@ show_copying_command (char *ignore, int from_tty)
   printf_filtered ("author or copyright holder as a result of your choosing to follow a\n");
   printf_filtered ("later version.\n");
   printf_filtered ("\n");
-  immediate_quit--;
 }
 
 static void
 show_warranty_command (char *ignore, int from_tty)
 {
-  immediate_quit++;
   printf_filtered ("  15. Disclaimer of Warranty.\n");
   printf_filtered ("\n");
   printf_filtered ("  THERE IS NO WARRANTY FOR THE PROGRAM, TO THE EXTENT PERMITTED BY\n");
@@ -643,7 +639,6 @@ show_warranty_command (char *ignore, int from_tty)
   printf_filtered ("Program, unless a warranty or assumption of liability accompanies a\n");
   printf_filtered ("copy of the Program in return for a fee.\n");
   printf_filtered ("\n");
-  immediate_quit--;
 }
 
 void
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index 5623e2d..93be085 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -888,6 +888,7 @@ print_partial_symbols (struct gdbarch *gdbarch,
   fprintf_filtered (outfile, "  %s partial symbols:\n", what);
   while (count-- > 0)
     {
+      QUIT;
       fprintf_filtered (outfile, "    `%s'", SYMBOL_LINKAGE_NAME (*p));
       if (SYMBOL_DEMANGLED_NAME (*p) != NULL)
 	{
@@ -1852,11 +1853,12 @@ print-psymbols takes an output file name and optional symbol file name"));
     perror_with_name (filename);
   make_cleanup_ui_file_delete (outfile);
 
-  immediate_quit++;
   ALL_PSYMTABS (objfile, ps)
-    if (symname == NULL || filename_cmp (symname, ps->filename) == 0)
-    dump_psymtab (objfile, ps, outfile);
-  immediate_quit--;
+    {
+      QUIT;
+      if (symname == NULL || filename_cmp (symname, ps->filename) == 0)
+	dump_psymtab (objfile, ps, outfile);
+    }
   do_cleanups (cleanups);
 }
 
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index d5a737b..d3028e6 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -85,17 +85,16 @@ print_symbol_bcache_statistics (void)
   struct program_space *pspace;
   struct objfile *objfile;
 
-  immediate_quit++;
   ALL_PSPACES (pspace)
     ALL_PSPACE_OBJFILES (pspace, objfile)
   {
+    QUIT;
     printf_filtered (_("Byte cache statistics for '%s':\n"), objfile->name);
     print_bcache_statistics (psymbol_bcache_get_bcache (objfile->psymbol_cache),
                              "partial symbol cache");
     print_bcache_statistics (objfile->macro_cache, "preprocessor macro cache");
     print_bcache_statistics (objfile->filename_cache, "file name cache");
   }
-  immediate_quit--;
 }
 
 void
@@ -106,10 +105,10 @@ print_objfile_statistics (void)
   struct symtab *s;
   int i, linetables, blockvectors;
 
-  immediate_quit++;
   ALL_PSPACES (pspace)
     ALL_PSPACE_OBJFILES (pspace, objfile)
   {
+    QUIT;
     printf_filtered (_("Statistics for '%s':\n"), objfile->name);
     if (OBJSTAT (objfile, n_stabs) > 0)
       printf_filtered (_("  Number of \"stab\" symbols read: %d\n"),
@@ -156,7 +155,6 @@ print_objfile_statistics (void)
     printf_filtered (_("  Total memory used for file name cache: %d\n"),
 		     bcache_memory_used (objfile->filename_cache));
   }
-  immediate_quit--;
 }
 
 static void
@@ -437,11 +435,12 @@ maintenance_print_symbols (char *args, int from_tty)
     perror_with_name (filename);
   make_cleanup_ui_file_delete (outfile);
 
-  immediate_quit++;
   ALL_SYMTABS (objfile, s)
-    if (symname == NULL || filename_cmp (symname, s->filename) == 0)
-    dump_symtab (objfile, s, outfile);
-  immediate_quit--;
+    {
+      QUIT;
+      if (symname == NULL || filename_cmp (symname, s->filename) == 0)
+	dump_symtab (objfile, s, outfile);
+    }
   do_cleanups (cleanups);
 }
 
@@ -663,13 +662,14 @@ maintenance_print_msymbols (char *args, int from_tty)
     perror_with_name (filename);
   make_cleanup_ui_file_delete (outfile);
 
-  immediate_quit++;
   ALL_PSPACES (pspace)
     ALL_PSPACE_OBJFILES (pspace, objfile)
-      if (symname == NULL || (!stat (objfile->name, &obj_st)
-			      && sym_st.st_ino == obj_st.st_ino))
-	dump_msymbols (objfile, outfile);
-  immediate_quit--;
+      {
+	QUIT;
+	if (symname == NULL || (!stat (objfile->name, &obj_st)
+				&& sym_st.st_ino == obj_st.st_ino))
+	  dump_msymbols (objfile, outfile);
+      }
   fprintf_filtered (outfile, "\n\n");
   do_cleanups (cleanups);
 }
@@ -682,11 +682,12 @@ maintenance_print_objfiles (char *ignore, int from_tty)
 
   dont_repeat ();
 
-  immediate_quit++;
   ALL_PSPACES (pspace)
     ALL_PSPACE_OBJFILES (pspace, objfile)
-      dump_objfile (objfile);
-  immediate_quit--;
+      {
+	QUIT;
+	dump_objfile (objfile);
+      }
 }