From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from loongson.cn (mail.loongson.cn [114.242.206.163]) by sourceware.org (Postfix) with ESMTP id D90003858413 for ; Fri, 18 Mar 2022 01:56:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D90003858413 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=loongson.cn Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=loongson.cn Received: from [10.130.0.135] (unknown [113.200.148.30]) by mail.loongson.cn (Coremail) with SMTP id AQAAf9DxfxPh5jNiVEQLAA--.9023S3; Fri, 18 Mar 2022 09:56:50 +0800 (CST) Subject: Re: [PATCH v3 2/4] gdb: testsuite: add new gdb_attach to check "attach" command To: Pedro Alves , Simon Marchi , gdb-patches@sourceware.org References: <1647525661-8607-1-git-send-email-yangtiezhu@loongson.cn> <1647525661-8607-3-git-send-email-yangtiezhu@loongson.cn> <0d25ccda-c26d-672f-6321-8a39aec9c1c6@palves.net> <25e1d92c-6b7e-9df3-640e-92227cf9e8ae@palves.net> From: Tiezhu Yang Message-ID: Date: Fri, 18 Mar 2022 09:56:49 +0800 User-Agent: Mozilla/5.0 (X11; Linux mips64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <25e1d92c-6b7e-9df3-640e-92227cf9e8ae@palves.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-CM-TRANSID: AQAAf9DxfxPh5jNiVEQLAA--.9023S3 X-Coremail-Antispam: 1UD129KBjvJXoWxGw1UAFyfCr1UWr1rGrykZrb_yoWrXFyDpa 1jqF1vyF4fXr4Iva17Aa18XayF9w4Fy3y5J34rJF1avF1DKFyxt3WUKwnYkrn3JrZ2q343 ZanIqw1fWry3AFDanT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDU0xBIdaVrnRJUUUvab7Iv0xC_Kw4lb4IE77IF4wAFF20E14v26r1j6r4UM7CY07I2 0VC2zVCF04k26cxKx2IYs7xG6rWj6s0DM7CIcVAFz4kK6r1j6r18M28lY4IEw2IIxxk0rw A2F7IY1VAKz4vEj48ve4kI8wA2z4x0Y4vE2Ix0cI8IcVAFwI0_tr0E3s1l84ACjcxK6xII jxv20xvEc7CjxVAFwI0_Gr1j6F4UJwA2z4x0Y4vEx4A2jsIE14v26r4UJVWxJr1l84ACjc xK6I8E87Iv6xkF7I0E14v26F4UJVW0owAS0I0E0xvYzxvE52x082IY62kv0487Mc02F40E FcxC0VAKzVAqx4xG6I80ewAv7VC0I7IYx2IY67AKxVWUXVWUAwAv7VC2z280aVAFwI0_Jr 0_Gr1lOx8S6xCaFVCjc4AY6r1j6r4UM4x0Y48IcVAKI48JMxk0xIA0c2IEe2xFo4CEbIxv r21lc2xSY4AK67AK6w4l42xK82IYc2Ij64vIr41l4I8I3I0E4IkC6x0Yz7v_Jr0_Gr1lx2 IqxVAqx4xG67AKxVWUJVWUGwC20s026x8GjcxK67AKxVWUGVWUWwC2zVAF1VAY17CE14v2 6r1Y6r17MIIYrxkI7VAKI48JMIIF0xvE2Ix0cI8IcVAFwI0_Jr0_JF4lIxAIcVC0I7IYx2 IY6xkF7I0E14v26r1j6r4UMIIF0xvE42xK8VAvwI8IcIk0rVW3JVWrJr1lIxAIcVC2z280 aVAFwI0_Jr0_Gr1lIxAIcVC2z280aVCY1x0267AKxVWUJVW8JbIYCTnIWIevJa73UjIFyT uYvjxUgTanUUUUU X-CM-SenderInfo: p1dqw3xlh2x3gn0dqz5rrqw2lrqou0/ X-Spam-Status: No, score=-6.0 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, NICE_REPLY_A, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 18 Mar 2022 01:56:54 -0000 On 03/18/2022 01:31 AM, Pedro Alves wrote: > On 2022-03-17 17:04, Simon Marchi via Gdb-patches wrote: > >> On 2022-03-17 12:50, Pedro Alves wrote: > >>> That looks odd to me -- it's not obvious that "pattern" is the option name. >>> >>> If we instead use parse_args, the option would be specified with "-pattern" (leading dash), >>> and we'd use the magic TCL args array, so we wouldn't need to wrap the options in a list. >>> Like so (on top of the whole series): >> >> I used parse_options because back when doing the rnglists support in the >> DWARF assembler, I had used parse_args and Tom Tromey mentioned he >> didn't really like that use of `args`, because it didn't make it clear >> what positional arguments were expected.. Unlike what you've done >> below, it was something like: >> >> proc rnglists { args } { >> # call parse_args, and then use the remaining args as positional >> # arguments >> } > > OK. I'd be fine with that pattern myself, FWIW, as I'd assume the real arguments would > be described in a comment, which you always have to look at anyhow, even if you have > positional arguments: > > # Usage: func FOO BAR [-opt OPT] > # > proc func { args } { > > > Maybe we could instead come up with a proc alternative that would > let you specify the options in the args list directly, like: > > proc_with_opts func { foo bar } { {opt1 "default1"} > {opt2 "default2"} } { > ... > } > > ... and we'd make it support users passing options before or after > positional args. > > Maybe that's going a bit too far, dunno. > > >> The way you did it here has the advantage that it's clear which >> positional arguments are expected. The downside is that flag arguments >> can't be put before positional arguments (like you usually can with >> shell or some tcl / expect procs). But I think that's an acceptable >> tradeoff, I'm fine with it. > > Yeah, it's what we do with gdb_test_multiple too, for example, even > thought we don't use parse_args there for some reason. > > # gdb_test_multiple COMMAND MESSAGE [ -prompt PROMPT_REGEXP] [ -lbl ] > # EXPECT_ARGUMENTS > ... > proc gdb_test_multiple { command message args } { > ... > >> >> Could we add a way to tell parse_args that unexpected arguments are not >> allowed, so it could throw the error itself (and therefore be re-used >> elsewhere)? > > I'd guess we could. I wouldn't make that a requirement for this patch, though. > Hi, Thank you very much for your discussions. I am not quite sure my understanding is correct. Which way is proper? parse_options or parse_args? Could you please clarify it for me? (1) parse_options gdb/testsuite/lib/gdb.exp proc gdb_attach { testpid {options {}} } { parse_options { {pattern ""} } gdb_test_multiple "attach $testpid" "attach" { -re -wrap "Attaching to.*ptrace: Operation not permitted\\." { unsupported "$gdb_test_name (Operation not permitted)" return 0 } -re -wrap "$pattern" { pass $gdb_test_name return 1 } } } gdb/testsuite/gdb.base/attach-pie-noexec.exp if { ![gdb_attach $testpid] } { kill_wait_spawned_process $test_spawn_id return } gdb/testsuite/gdb.base/jit-elf.exp set attach_opts { "pattern" "main.*at .*$::main_srcfile:.*" } if { ![gdb_attach $testpid $attach_opts] } { return 0 } (2) parse_args gdb/testsuite/lib/gdb.exp proc gdb_attach { testpid args } { parse_args { {pattern ""} } if { [llength $args] != 0 } { error "Unexpected arguments: $args" } gdb_test_multiple "attach $testpid" "attach" { -re -wrap "Attaching to.*ptrace: Operation not permitted\\." { unsupported "$gdb_test_name (Operation not permitted)" return 0 } -re -wrap "$pattern" { pass $gdb_test_name return 1 } } } gdb/testsuite/gdb.base/attach-pie-noexec.exp if { ![gdb_attach $testpid] } { kill_wait_spawned_process $test_spawn_id return } gdb/testsuite/gdb.base/jit-elf.exp if { ![gdb_attach $testpid \ -pattern "main.*at .*$::main_srcfile:.*"] } { return 0 } I will wait for your feedback and then send v4 later. Thanks, Tiezhu