From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8998 invoked by alias); 26 Jul 2012 16:56:43 -0000 Received: (qmail 8981 invoked by uid 22791); 26 Jul 2012 16:56:39 -0000 X-SWARE-Spam-Status: No, hits=-5.7 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN,KHOP_RCVD_UNTRUST,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,SPF_HELO_PASS,TW_BJ,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 26 Jul 2012 16:56:25 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q6QGuNUW025148 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 26 Jul 2012 12:56:24 -0400 Received: from barimba (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q6QGuLTE027697 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Thu, 26 Jul 2012 12:56:22 -0400 From: Tom Tromey To: Jan Kratochvil Cc: gdb-patches@sourceware.org, Doug Evans Subject: Re: [patch] Remove one immediate_quit++-- (in auto-load.c) References: <20120726135446.GA15749@host2.jankratochvil.net> Date: Thu, 26 Jul 2012 16:56:00 -0000 In-Reply-To: <20120726135446.GA15749@host2.jankratochvil.net> (Jan Kratochvil's message of "Thu, 26 Jul 2012 15:54:46 +0200") Message-ID: <87boj2folm.fsf@fleche.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-07/txt/msg00626.txt.bz2 >>>>> "Jan" == Jan Kratochvil 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 * 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. \n"); + printf_filtered (" Copyright (C) 2007, 2012 Free Software Foundation, Inc. \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); + } }