From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 0C14C3858D32 for ; Mon, 16 Jan 2023 21:07:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0C14C3858D32 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=polymtl.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=polymtl.ca Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 30GL77oW019221 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 16 Jan 2023 16:07:12 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 30GL77oW019221 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=default; t=1673903233; bh=F2z6zGIZ0/Dq5gSKjX2rEQUz01AkkC0lCSJM6O1fyYY=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=C6QBLkPcpyTti3IsJk80/5NPb29oLMePX4Gfy2fa18vGwFsqiGn72okeOPKLfeZU6 lKtW9t4pw82Y+Ci11JWm9kpwqgHFX2b1J3eSTcJi8Kdo01nAM4NqAkfCdNfst1Rr+B 3WWzQpFX1I2Gi0ZT0E3M3E1+yw1FVdW1J23o8FR0= Received: from [10.0.0.11] (unknown [217.28.27.60]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 29D8E1E110; Mon, 16 Jan 2023 16:07:07 -0500 (EST) Message-ID: <4a937b46-d937-619d-419d-93daefb92927@polymtl.ca> Date: Mon, 16 Jan 2023 16:07:06 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Subject: Re: [PATCH v8 6/6] GDB/testsuite: Expand for character string limiting options Content-Language: en-US To: "Maciej W. Rozycki" , gdb-patches@sourceware.org Cc: Andrew Burgess , Tom Tromey , Simon Sobisch References: From: Simon Marchi In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Mon, 16 Jan 2023 21:07:07 +0000 X-Spam-Status: No, score=-3031.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,KAM_ASCII_DIVIDERS,KAM_SHORT,NICE_REPLY_A,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi, I noted a few minor comments, once they are fixed this LGTM: Approved-By: Simon Marchi >From my point of view, there's not need to send a new version. > Index: src/gdb/testsuite/gdb.base/printcmds.exp > =================================================================== > --- src.orig/gdb/testsuite/gdb.base/printcmds.exp > +++ src/gdb/testsuite/gdb.base/printcmds.exp > @@ -451,11 +451,11 @@ proc test_print_all_chars {} { > # Test interaction of the number of print elements to print and the > # repeat count, set to the default of 10. > > -proc test_print_repeats_10 {} { > +proc test_print_repeats_10_one { setting } { Please expand the doc to say what SETTING is. > Index: src/gdb/testsuite/gdb.pascal/str-chars.exp > =================================================================== > --- /dev/null > +++ src/gdb/testsuite/gdb.pascal/str-chars.exp > @@ -0,0 +1,56 @@ > +# Copyright 2022 Free Software Foundation, Inc. > +# > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + > +load_lib "pascal.exp" > + > +standard_testfile .pas > + > +if {[gdb_compile_pascal "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug ]] != "" } { > + return -1 > +} > + > +clean_restart ${testfile} > +gdb_breakpoint ${srcfile}:[gdb_get_line_number "set breakpoint 1 here"] > + > +# Verify that "start" lands inside the right procedure. > +if { [gdb_start_cmd] < 0 } { > + untested start > + return -1 > +} Can you update this to use runto_main, mirroring what was done in this commit? https://gitlab.com/gnutools/binutils-gdb/-/commit/4a6bdfb9baa27e29151c7e97ae2abbe902f53638 > Index: src/gdb/testsuite/gdb.python/py-format-string.exp > =================================================================== > --- src.orig/gdb/testsuite/gdb.python/py-format-string.exp > +++ src/gdb/testsuite/gdb.python/py-format-string.exp > @@ -705,13 +705,13 @@ proc_with_prefix test_static_members {} > } > > # Test the max_elements option for gdb.Value.format_string. > -proc_with_prefix test_max_elements {} { > +proc test_max_string_one { setting unlimited } { The comment above should be expanded a bit. > @@ -722,8 +722,10 @@ proc_with_prefix test_max_elements {} { > check_format_string "a_binary_string" $opts > check_format_string "a_binary_string_array" $opts > check_format_string "a_big_string" $opts > - check_format_string "an_array" $opts > - check_format_string "an_array_with_repetition" $opts > + if { $setting == "elements"} { Space before closing bracket (just for consistency with the opening one). > @@ -785,15 +791,24 @@ proc_with_prefix test_max_elements {} { > } > } > > - with_temp_option "set print elements 4" "set print elements 200" { > + with_temp_option "set print $setting 4" "set print $setting 200" { > check_format_string "a_string" "" \ > "${default_pointer_regexp} \"hell\"..." > check_format_string "a_binary_string" "" \ > "${default_pointer_regexp} \"hell\"..." > check_format_string "a_binary_string_array" "" \ > "\"hell\"..." > - check_format_string "an_array_with_repetition" "" \ > - "\\{1, 3 ...\\}" > + if { $setting == "elements"} { > + check_format_string "an_array_with_repetition" "" \ > + "\\{1, 3 ...\\}" > + } > + } > +} > + > +proc_with_prefix test_max_string {} { Is the name (and the name of test_max_string_one) accurate? Does it test only strings, or strings and arrays? Just wondering if test_max_strings_and_arrays would be more accurate. > + foreach_with_prefix setting { "elements" "characters" } { The foreach_with_prefix here is not really necessary, because all tests under test_max_string_one are under a proc_with_prefix that depends on the setting passed. You can remove it if you want to make test names a bit shorter, but I'm fine with leaving it as well for extra clarity. > + test_max_string_one $setting \ > + [string map {elements 0 characters 4294967295} $setting] Can you put a comment here and/or in test_max_string_one's comment, explaining what the last parameter does? It's not clear to me what is done. Simon