From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7079 invoked by alias); 10 Dec 2003 17:56:58 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 7071 invoked from network); 10 Dec 2003 17:56:56 -0000 Received: from unknown (HELO localhost.redhat.com) (216.129.200.20) by sources.redhat.com with SMTP; 10 Dec 2003 17:56:56 -0000 Received: from gnu.org (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id 3BF2B2B8F; Wed, 10 Dec 2003 12:56:52 -0500 (EST) Message-ID: <3FD75E64.1020508@gnu.org> Date: Wed, 10 Dec 2003 17:56:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-US; rv:1.0.2) Gecko/20030820 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Nick Roberts Cc: gdb-patches@sources.redhat.com Subject: Re: RFC (gdb/mi): -stack-list-locals + REVISED PATCH References: <16319.64137.458928.417189@nick.uklinux.net> <3FC3F85F.8050007@gnu.org> <16332.423.456414.834703@nick.uklinux.net> <16341.13503.256676.933542@nick.uklinux.net> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2003-12/txt/msg00295.txt.bz2 > This patch: > > 1) Follows Jim Ingham's advice of using check_typedef to guard against the > case of TYPE_CODE_TYPEDEF. > > 2) Avoids a segmentation fault if -stack-list-locals is invoked before the > inferior has started execution. > > 3) Still introduces "-stack-list-locals 2". I'm not sure how to simplify this > as any change must presumably be beackward compatible. I don't think using > different numbers to mean different things is a problem here as mi commands > are not intended for the user and so don't need to be remembered by > him/her. True, but they also need to be fairly self documenting. Anyway, see below: > Nick http://www.nick.uklinux.net > > > > *** mi-cmd-stack.c.~1.19.~ 2003-06-12 23:29:37.000000000 +0100 > --- mi-cmd-stack.c 2003-12-09 02:12:45.000000000 +0000 > *************** > *** 140,145 **** > --- 140,148 ---- > if (argc != 1) > error ("mi_cmd_stack_list_locals: Usage: PRINT_VALUES"); > > + if (!deprecated_selected_frame) > + error ("No frame selected."); struct frame_info *frame; ... frame = get_selected_frame (); is better. It throws an error if there is no frame, and follow on code can use "frame" instead of "deprecated_selected_frame" (and would be a really appreciated cleanup!). > list_args_or_locals (1, atoi (argv[0]), deprecated_selected_frame); I was thinking of something as simple as: enum print_values { PRINT_NO_VALUES, PRINT_ALL_VALUES, PRINT_SIMPLE_VALUES }; and then the very mechanical: enum print_values print_values; if (strcmp (argv[0], "0") == 0 || strcmp (argv[0], "no-values") == 0) print_values = PRINT_NO_VALUES; else if (strcmp (argv[0], "1") == 0 || strcmp (argv[0], "all-values") == 0) print_values = PRINT_ALL_VALUES; else if (strcmp (argv[0], "simple-values") == 0) print_values = PRINT_SIMPLE_VALUES; else error ("..."); list_args_or_locals (1, print_values, ...); (the names aren't the best so feel free to improve). > return MI_CMD_DONE; > } > *************** > *** 273,288 **** > make_cleanup_ui_out_tuple_begin_end (uiout, NULL); > ui_out_field_string (uiout, "name", DEPRECATED_SYMBOL_NAME (sym)); > > ! if (values) if (values != PRINT_NO_VALUES) > ! { > ! struct symbol *sym2; > ! if (!locals) > ! sym2 = lookup_symbol (DEPRECATED_SYMBOL_NAME (sym), > ! block, VAR_DOMAIN, > ! (int *) NULL, > ! (struct symtab **) NULL); > ! else > sym2 = sym; > print_variable_value (sym2, fi, stb->stream); > ui_out_field_stream (uiout, "value", stb); > do_cleanups (cleanup_tuple); > --- 276,304 ---- > make_cleanup_ui_out_tuple_begin_end (uiout, NULL); > ui_out_field_string (uiout, "name", DEPRECATED_SYMBOL_NAME (sym)); > > ! struct symbol *sym2; > ! if (!locals) > ! sym2 = lookup_symbol (DEPRECATED_SYMBOL_NAME (sym), Since you're here, I'd change this to SYMBOL_PRINT_NAME. The comment below, from symtab.h, hopefully explains the difference (I also hope I picked the correct winner): /* Now come lots of name accessor macros. Short version as to when to use which: Use SYMBOL_NATURAL_NAME to refer to the name of the symbol in the original source code. Use SYMBOL_LINKAGE_NAME if you want to know what the linker thinks the symbol's name is. Use SYMBOL_PRINT_NAME for output. Use SYMBOL_DEMANGLED_NAME if you specifically need to know whether SYMBOL_NATURAL_NAME and SYMBOL_LINKAGE_NAME are different. Don't use DEPRECATED_SYMBOL_NAME at all: instances of that macro should be replaced by SYMBOL_NATURAL_NAME, SYMBOL_LINKAGE_NAME, or perhaps SYMBOL_PRINT_NAME. */ > ! block, VAR_DOMAIN, > ! (int *) NULL, > ! (struct symtab **) NULL); > ! else > sym2 = sym; > + if (values == 2) if (values == PRINT_SIMPLE_VALUES) > + { > + type_print (sym2->type, "", stb->stream, -1); > + ui_out_field_stream (uiout, "type", stb); > + if (TYPE_CODE (check_typedef (sym2->type)) != TYPE_CODE_ARRAY > + && > + TYPE_CODE (check_typedef (sym2->type)) != TYPE_CODE_STRUCT) You may also want to consider TYPE_CODE_UNION? Your choice. > + { > + print_variable_value (sym2, fi, stb->stream); > + ui_out_field_stream (uiout, "value", stb); > + } > + do_cleanups (cleanup_tuple); > + } > + else if (values) if (values == PRINT_ALL_VALUES) > + { > print_variable_value (sym2, fi, stb->stream); > ui_out_field_stream (uiout, "value", stb); > do_cleanups (cleanup_tuple); > Anyway, it's basicly there. Just the doco update (which is eli's call) (and for the assignment to pop up), and testcase. For the testcase, just edit mi-stack.exp. You don't need to edit mi[12]-stack.exp as there's no expectation that this new feature will work with old MI versions. (since I'm traveling, I may be slow in responding, hopefully though, between my self and elena someone will be able to give a thumbs up and organize your account). Andrew