From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id e1XsBj92u2pdfhYAWB0awg (envelope-from ) for ; Tue, 29 Sep 2026 04:26:39 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=caWKSYxT; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id ECEE61E033; Tue, 29 Sep 2026 04:26:38 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-6.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [IPv6:2620:52:6:3111::32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 77D811E033 for ; Tue, 29 Sep 2026 04:26:37 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4673E4BB24CD for ; Tue, 29 Sep 2026 08:26:29 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4673E4BB24CD Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=caWKSYxT Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id BDED44BA23CE for ; Tue, 29 Sep 2026 08:26:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BDED44BA23CE Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org BDED44BA23CE Authentication-Results: sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1790670362; cv=none; b=k7psB001r8aJjuW0vU9+rzhZ3Lp7lsxMniD4FkTTUYcx4QC76wL+zAf9fMR+JNmKdTAj3fzjVEIiI8e12B7cCDYM71/hq9XKDmYqbeTMqiR2XndWLqTlsX5JHVdgB7skBXYLKVh9ZnxkeXWz6q4UWhKQETAW1QM1Y4GrvmGNtKI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1790670362; c=relaxed/simple; bh=BB8+QXzYdB+br+EqnaoQEMbdeOOWjbB3Yvhw02BMMtk=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=QUgZk7qIkYX1FqiFkzRE8oVUtZubf9vbfcuvrf3Snv/nf+x3i0uTRQgmG/eW6JJj9EB7mz2lrGEVixTHDwEVGSQ7gfWgY6x8fOxEWvCqfzss+aDlHqB9JX5Y7oydB8m02qaFEmeTxtaN0RDswu900GE69O69c5oP13tFSvpwqaQ= ARC-Authentication-Results: i=1; sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=caWKSYxT DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BDED44BA23CE DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1790670361; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=UNldaEwtp7c+sZTav+3nl+mQie8VKlSbDOlQza5T3ow=; b=caWKSYxT5Hju0UMfOi/hL05ZBCvcxLX7F6M3Zny7n+vTAYge92LUFX9vd1BnOE0daubRIL LhdDtUWEmBcQvZmreVwdxDQeqp+DgmJAGM/JhA/wz3bnNKbwhPj82COIBGrZQzelI7O+ib uCBVVq3eeHiaZEy5pmHuZSSh+73ndVw= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-441-4QFMNGKJPx25QxtCliNkjw-1; Tue, 29 Sep 2026 04:26:00 -0400 X-MC-Unique: 4QFMNGKJPx25QxtCliNkjw-1 X-Mimecast-MFC-AGG-ID: 4QFMNGKJPx25QxtCliNkjw_1790670359 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-49e635a6002so45901965e9.0 for ; Tue, 29 Sep 2026 01:26:00 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1790670359; x=1791275159; h=content-type:mime-version:message-id:date:references:in-reply-to :subject:to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to:content-type; bh=UNldaEwtp7c+sZTav+3nl+mQie8VKlSbDOlQza5T3ow=; b=nNpARAVlPWHvKq4UdopQbnQy97WkMi/2wvciwH7dM2Ne9ltOYfve190D7xUh+f/scO H3lkzMwApumdgqysrbGtYyJ5V1zkWNDpP7earaQGBKdNxDkXJmzZ3MEJOGieYGDGYGBF ifOOYavO1w72Ic+sO5BgMFQDqG/6I7WuWYxjn9XCF+2vz9Sdqm65/C14wEliQGvNIKqp h+fetgpBPMpM7+v19S7e9VUbXZZ5sC2smr2SquNmFGOUuoJqUP8HE400PW/2fUyc283Z iR9qE4sruU58CFln4pFGF0s8tuOCFfiyyDoCCeshL7RY5QAFaFLXNJDTYKo3BCnM/047 vh6g== X-Forwarded-Encrypted: i=1; AKwUvBzeHiXiejb03ybd+PmruHVhvtXFRYaxiq15U7B9g2lVVE+jt7n/mDDKKSRAapD2I+SpwCdqBZvrVcofoA==@sourceware.org X-Gm-Message-State: AFuF++lJR22SZoIBI13lZwX8wz9TQ0VbODtQRHRx/Mi7uYFp3L3PlpqL 3C3d/EeJfQN6oF1lnj5D1Zd+OApK7O7lWMBlFz9qNxFMjByt0KC5P/a2wqCNqnHRSIWwujXewb/ ct1SODO3/+THu9zF4hRMkxLj9/C2Xq1beKZwI2ktt9ztDHAE27iWlI9aB47tzOh4Q/XVsALM= X-Gm-Gg: AYBFou1sIYGep4EUmT9siCiH2uVyp1+GXI+AtApoqk8K+jBHiSgMBsUW5DOdfTHosRv WGosIIUZXrLc1ReScy720vUo5mtHBlRdi3TCW9o/z3fAqTGCKFIGAzUTZOgcJCDv787guQ1+ZQm rYIWMogi91nvq+RcQsNB/uho13nLXqxMn7nL8bsLIgu7P6BriqS4LQe4Sy6TV2LkdSfXtjs5zT0 XoqjlSv05MXpFwBxfLCLE1S3d4yUc/3CgP2ihRVPzVwWsX4iXd/PHf8Ge2SQ5oLHOMV0BFDU5OR QSA+530M8yzo+ki5wwLEY7y0tQhvoO4WLGbRqAAYyHEYSUeouzpZ8Cad1IW4M7ymMmuG X-Received: by 2002:a05:600c:8b71:b0:4a0:328:43df with SMTP id 5b1f17b1804b1-4a0032847fcmr78885135e9.21.1790670358711; Tue, 29 Sep 2026 01:25:58 -0700 (PDT) X-Received: by 2002:a05:600c:8b71:b0:4a0:328:43df with SMTP id 5b1f17b1804b1-4a0032847fcmr78884785e9.21.1790670358194; Tue, 29 Sep 2026 01:25:58 -0700 (PDT) Received: from localhost ([213.31.44.29]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4a00cfec770sm63259795e9.8.2026.09.29.01.25.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 29 Sep 2026 01:25:56 -0700 (PDT) From: Andrew Burgess To: Tom de Vries , gdb-patches@sourceware.org Subject: Re: [PATCH v2 1/2] [gdb/testsuite] Simplify core_find In-Reply-To: <20260927054254.1986148-2-tdevries@suse.de> References: <20260927054254.1986148-1-tdevries@suse.de> <20260927054254.1986148-2-tdevries@suse.de> Date: Tue, 29 Sep 2026 09:25:55 +0100 Message-ID: <875wzo1n1o.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: BH6p5lCf0b621VHBWQ88wIFGoRKeVtFq5VkpXBKdvwo_1790670359 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org Tom de Vries writes: > In proc core_find we have: > ... > catch "system \"(cd ${coredir}; ulimit -c unlimited; $coredump_filter_cmd; ${binfile} ${arg}; true) >${output_file} 2>&1\"" > ... > > We can rewrite this into something more readable with less quote and escape > magic: I'm not opposed to this change, but I don't understand the motivation. Is the "magic" to which you refer the \" ? Or maybe you getting ahead of yourself and referencing the change in gdb.base/corefile-exec-context.exp, which does seem like a nice cleanup. To me, the original core_find code was clearer, but the extra complexity seems worth if for the improvements possible in the test scripts. > ... > set arg [subst -nocommands -novariables $arg] > set cmd [subst_vars { > (cd ${coredir}; > ulimit -c unlimited; > $coredump_filter_cmd; > ${binfile} ${arg}; > true) \ > >${output_file} 2>&1}] > catch { > system $cmd > } > ... > > The "set arg [subst ... $arg]" is a bit awkward, and dropping it allows us to > update test-case gdb.base/corefile-exec-context.exp to use a bit more typical > setup with string_to_regex. typo: string_to_regex -> string_to_regexp Thd discussion of "set arg [subst ... $arg]" confusing. You introduce it, then say it's dropped, but never explain why it was introduced, you just assume the reason is self-evident. It's not. At least, not to me. I think you should probably just remove that line and explain why the new code is better -- this would be great as others (me) could read your commit and learn from it. > > Note that what is being tested hasn't changed. > > We can print the effective value of arg by adding to the command passed to > system. Without this patch using: > ... > echo \\\"$arg\\\; > ... > and with this patch using: > ... > echo "$arg"; > ... > and in both cases we get: > ... > aaaaa bbbbb ccccc ddddd e\ e\ e\ e\ e > ... > --- > .../gdb.base/corefile-exec-context.exp | 13 +++++++--- > gdb/testsuite/lib/gdb.exp | 26 ++++++++++++++++--- > 2 files changed, 31 insertions(+), 8 deletions(-) > > diff --git a/gdb/testsuite/gdb.base/corefile-exec-context.exp b/gdb/testsuite/gdb.base/corefile-exec-context.exp > index 9b018533b68..56c68a6a5cd 100644 > --- a/gdb/testsuite/gdb.base/corefile-exec-context.exp > +++ b/gdb/testsuite/gdb.base/corefile-exec-context.exp > @@ -69,7 +69,7 @@ gdb_test_multiple "core-file $corefile_1" "load core file no args" { > } > > # Generate a core file, this time pass some arguments to the inferior. > -set args "aaaaa bbbbb ccccc ddddd e\\\\ e\\\\ e\\\\ e\\\\ e" > +set args {aaaaa bbbbb ccccc ddddd e\ e\ e\ e\ e} > set corefile [core_find $binfile {} $args] > if {$corefile == ""} { > untested "unable to create corefile" > @@ -82,8 +82,11 @@ remote_exec build "mv $corefile $corefile_2" > # argument list are seen. > clean_restart $testfile > set saw_generated_line false > +set re_args [string_to_regexp $args] > +set re_cmd "[string_to_regexp $binfile] $re_args" > +set re_line [subst_vars {^Core was generated by `$re_cmd'\.\r\n}] > gdb_test_multiple "core-file $corefile_2" "load core file with args" { > - -re "^Core was generated by `[string_to_regexp $binfile] $args'\\.\r\n" { > + -re $re_line { > set saw_generated_line true > exp_continue > } > @@ -99,7 +102,8 @@ gdb_test_multiple "core-file $corefile_2" "load core file with args" { > > # Also, the argument list should be available through 'show args'. > gdb_test "show args" \ > - "Argument list to give program being debugged when it is started is \"$args\"\\." > + [subst_vars \ > + {Argument list to give program being debugged when it is started is "$re_args"\.}] > > # Move up to 'main'. Do it this way because we cannot know how many > # frames up 'main' actually is. > @@ -178,8 +182,9 @@ proc check_for_env_var { var_name var_value } { > gdb_assert { ![check_for_env_var $env_var_name $env_var_value] } \ > "environment variable is not set before core file load" > > +set re_cmd "[string_to_regexp $binfile] $re_args" I don't think this line is needed. Has RE_CMD changed since it was first computed? I don't think BINFILE has, and RE_ARGS is new, and only computed the once above. If this line is necessary then maybe a comment explaining why would be good. Thank, Andrew > gdb_test "core-file $corefile_3" \ > - "Core was generated by `[string_to_regexp $binfile] $args'\\.\r\n.*" \ > + [subst_vars {Core was generated by `$re_cmd'\.\r\n.*}] \ > "load core file for environment test" > > gdb_assert { [check_for_env_var $env_var_name $env_var_value] } \ > diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp > index 9ade9a16818..2cfdbdda09d 100644 > --- a/gdb/testsuite/lib/gdb.exp > +++ b/gdb/testsuite/lib/gdb.exp > @@ -10312,8 +10312,18 @@ proc core_find {binfile {deletefiles {}} {arg ""} {output_file "/dev/null"}} { > } > } > > - # tclint-disable command-args > - catch "system \"(cd ${coredir}; ulimit -c unlimited; $coredump_filter_cmd; ${binfile} ${arg}; true) >${output_file} 2>&1\"" > + set cmd [subst_vars { > + (cd ${coredir}; > + ulimit -c unlimited; > + $coredump_filter_cmd; > + ${binfile} ${arg}; > + true) \ > + >${output_file} 2>&1}] > + verbose -log "Executing on build: $cmd" > + catch { > + system $cmd > + } > + > # remote_exec host "${binfile}" > set binfile_basename [file tail $binfile] > foreach i [list \ > @@ -10343,8 +10353,16 @@ proc core_find {binfile {deletefiles {}} {arg ""} {output_file "/dev/null"}} { > # ulimit here if we didn't find a core file above. > # Oh, I should mention that any "braindamaged" non-Unix system has > # the same problem. I like the cd bit too, it's really neat'n stuff. > - # tclint-disable command-args > - catch "system \"(cd ${objdir}/${subdir}; ${binfile}; true) >/dev/null 2>&1\"" > + set cmd [subst_vars { > + (cd ${objdir}/${subdir}; > + ${binfile}; > + true) \ > + >/dev/null 2>&1}] > + verbose -log "Executing on build: $cmd" > + catch { > + system $cmd > + } > + > foreach i "${objdir}/${subdir}/core ${objdir}/${subdir}/core.coremaker.c ${binfile}.core" { > if {[remote_file build exists $i]} { > remote_exec build "mv $i $destcore" > -- > 2.51.0