From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 33365 invoked by alias); 30 Nov 2017 10:57:56 -0000 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 Received: (qmail 33351 invoked by uid 89); 30 Nov 2017 10:57:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=BAYES_00,KB_WAM_FROM_NAME_SINGLEWORD,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy=Hx-languages-length:3863 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 30 Nov 2017 10:57:54 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AB1A07F41E for ; Thu, 30 Nov 2017 10:57:53 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 01D8F5D6A9; Thu, 30 Nov 2017 10:57:50 +0000 (UTC) Subject: Re: [PATCH v2] Make '{add-,}symbol-file' not care about the position of command line arguments To: Sergio Durigan Junior , GDB Patches References: <20171129214451.14257-1-sergiodj@redhat.com> <20171130042448.30882-1-sergiodj@redhat.com> From: Pedro Alves Message-ID: <2fba79f1-e16e-38f2-bc0f-1c27da05e5c1@redhat.com> Date: Thu, 30 Nov 2017 10:57:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20171130042448.30882-1-sergiodj@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-11/txt/msg00827.txt.bz2 On 11/30/2017 04:24 AM, Sergio Durigan Junior wrote: > Changes from v1: > > - Commit message has been rewritten. > > - Implemented position-independent argument parsing for > 'add-symbol-file'. > > - Added testcases. Looks like you missed the comment about "--". Take a look at maintenance_print_symbols for an example of a command that supports ending options with "--". Can you add that while you're at it, please? For a test, I'd suggest e.g., "symbol-file -- -non-existent-file" and confirming gdb errors out. That's simpler than actually creating a file. > + if (*arg != '-') > { > - /* It's an option (starting with '-') or it's an argument > - to an option. */ > if (expecting_sec_name) > { > sect_opt sect = { arg, NULL }; > sect_opts.push_back (sect); > - expecting_sec_name = 0; > + expecting_sec_name = false; > } > else if (expecting_sec_addr) > { > sect_opts.back ().value = arg; > - expecting_sec_addr = 0; > + expecting_sec_addr = false; > } > - else if (strcmp (arg, "-readnow") == 0) > - flags |= OBJF_READNOW; > - else if (strcmp (arg, "-s") == 0) > + else if (filename == NULL) > + { > + /* First non-option argument is always the filename. */ > + filename.reset (tilde_expand (arg)); > + } > + else if (!seen_addr) > { > - expecting_sec_name = 1; > - expecting_sec_addr = 1; > + /* The second non-option argument is always the text > + address at which to load the program. */ > + sect_opt sect = { ".text", arg }; > + sect_opts.push_back (sect); > + seen_addr = true; Does this push_back directly here mean that these two commands end up with different semantics? (gdb) add-symbol-file FILE 0 -s .text 0x1000 (gdb) add-symbol-file -s .text 0x1000 FILE 0 Not sure that's a good idea. Please add a test with "-s .text"... > +# Check that we can pass parameters using any position in the command > +# line. > +gdb_test "add-symbol-file -readnow $binfile 0x0 -s .bss 0x3" \ > + "Not confirmed\." \ > + "add-symbol-file positionless -readnow" \ > + "add symbol table from file \"${binfile}\" at\r\n\t\.text_addr = 0x0\r\n\t\.bss_addr = 0x3\r\n\\(y or n\\) " \ > + "n" > +# When we use -s as the first argument, the section will be printed > +# first as well. > +gdb_test "add-symbol-file -s .bss 0x3 -readnow $binfile 0x0" \ > + "Not confirmed\." \ > + "add-symbol-file positionless -s" \ > + "add symbol table from file \"${binfile}\" at\r\n\t\.bss_addr = 0x3\r\n\t\.text_addr = 0x0\r\n\\(y or n\\) " \ > + "n" > +gdb_test "add-symbol-file $binfile 0x0 -s .bss 0x3" \ > + "Not confirmed\." \ > + "add-symbol-file positionless -s, no -readnow" \ > + "add symbol table from file \"${binfile}\" at\r\n\t\.text_addr = 0x0\r\n\t\.bss_addr = 0x3\r\n\\(y or n\\) " \ > + "n" Using a number != 0x0 is a little better, since its easy for a variable to end up always zero-initialized / zero-propagated by mistake, and the test wouldn't notice. > +# Since we're here, might as well test the 'symbol-file' command and > +# if its arguments can also be passed at any position. > +gdb_test "symbol-file -readnow $binfile" \ > + "Reading symbols from ${binfile}\.\.\.expanding to full symbols\.\.\.done\." \ > + "symbol-file with -readnow first" > +gdb_exit > +gdb_start > +gdb_reinitialize_dir $srcdir/$subdir Just use clean_restart with no argument. > +gdb_test "symbol-file $binfile -readnow" \ > + "Reading symbols from ${binfile}\.\.\.expanding to full symbols\.\.\.done\." \ > + "symbol-file with -readnow second" > +gdb_test "symbol-file -readnow" \ > + "no symbol file name was specified" \ > + "symbol-file without filename" > + > +gdb_exit > +gdb_start > +gdb_reinitialize_dir $srcdir/$subdir Ditto. > + > gdb_test "add-symbol-file ${binfile} 0 -s" \ > "Missing section name after .-s." \ > "add-symbol-file bare -s" > Thanks, Pedro Alves