From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x236.google.com (mail-lj1-x236.google.com [IPv6:2a00:1450:4864:20::236]) by sourceware.org (Postfix) with ESMTPS id 4CF333858C52 for ; Thu, 19 Jan 2023 21:18:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4CF333858C52 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=embecosm.com Received: by mail-lj1-x236.google.com with SMTP id o7so3493788ljj.8 for ; Thu, 19 Jan 2023 13:18:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=mime-version:user-agent:references:message-id:in-reply-to:subject :cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=bWH2u2RUiEq8ThGmfOrhLXTFr+Yx6PhElKUf5QoQumg=; b=PHYRbqUfup9ttftCC0ZF1EbHjqhyh+tqLfKJfarJi1D848dYPMQY4DFmY6S6WlGxHO L/lI9piOf51pZNrPUDKHpm8CP2LcAIsiUgrzUjwjDxYZlYrFX1opOgRjoV0C8vYO3aub cXPQw6toN+TbQ3oomOL3jyDu7OHmoLiar21XVz+sK/AyVEdEtD3jHGFh7bMIPU9Nc2H7 dgNxdNEJDYNAHbmk/LBZVpN4iTnETQlr7qemnSIPDgnPCddqO5EfE1AChJ+4GM76KzLE 3ngkrukJuuBr8KRm5EkRf4uRpzfHH+MU7j9wO1PtO9IY4PqWJ/vk9afHuk3yLhMpRpRq W5pw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:user-agent:references:message-id:in-reply-to:subject :cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=bWH2u2RUiEq8ThGmfOrhLXTFr+Yx6PhElKUf5QoQumg=; b=qy6l2nY+MckOeOQtHQukT4drU7+KvLsp6Qi+0CE4WTXI6vFPhTSoOxmZK8BJ4R2Y4S P5Di9gFoyCUT8NbMexeuKtJQoDUp/8zzDHJMKHCKrbQmls+tPji5f/3yatxFL3AcKEm/ 5voRK7AYdFKaxWvVp0T0u6o/F6R5tR1li6vim/yz9hiEhiaK7QgVfIUbtm0Fc1hokD8S +jeE7SB/v+hBAlECtVvDLlhEJJ45pfH9gryRydbZ0gXrAuHXbA5PdQHELxalE3GrtrvN rbfSISUTmyYxa6g6R5WUMvsm0AJ+l5sAg/5ErO11wpFEnqoP9Lay3VR+F7sdSF/uPauG 2lWw== X-Gm-Message-State: AFqh2ko9HCmEisdRZ/pkgkZd1wJmCKFsGPTM3fdP3Miks2oX9bhhu57r hXite3GPlCn2/s8es2SzbfDICg== X-Google-Smtp-Source: AMrXdXsqurvkCgreHjn4tUj0MS3m+pRia3m8iOoYPoij0dcEOLYhgDZQ9Ff+rdHJJ0wMQgVEfNp/zQ== X-Received: by 2002:a2e:9e1a:0:b0:28b:a20c:250a with SMTP id e26-20020a2e9e1a000000b0028ba20c250amr2557989ljk.47.1674163107897; Thu, 19 Jan 2023 13:18:27 -0800 (PST) Received: from [192.168.219.3] ([78.8.192.131]) by smtp.gmail.com with ESMTPSA id j20-20020a2e8514000000b00286dbab9a87sm1984218lji.91.2023.01.19.13.18.20 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 19 Jan 2023 13:18:23 -0800 (PST) Date: Thu, 19 Jan 2023 21:18:19 +0000 (GMT) From: "Maciej W. Rozycki" To: Simon Marchi cc: gdb-patches@sourceware.org, Andrew Burgess , Tom Tromey , Simon Sobisch Subject: Re: [PATCH v8 6/6] GDB/testsuite: Expand for character string limiting options In-Reply-To: <4a937b46-d937-619d-419d-93daefb92927@polymtl.ca> Message-ID: References: <4a937b46-d937-619d-419d-93daefb92927@polymtl.ca> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-2.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Mon, 16 Jan 2023, Simon Marchi wrote: > > # 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. Done. > > +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 Done. > > # 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. Done. > > - 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). Fixed, thanks. > > - 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. Ultimately it verifies `gdb.Value.format_string', so I think the name is accurate enough, even though with a couple of test cases we do use it with non-string arrays. > > + 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. I've left it for consistency with `test_max_string_one', which also uses redundant `with_test_prefix' invocations throughout, so we have e.g.: PASS: gdb.python/py-format-string.exp: format_string: lang_c: test_all_common: test_max_string: setting=elements: max_elements=200: a_point_t with option max_elements=200 where both "setting=elements: " and "max_elements=200: " could be removed, because we have "with option max_elements=200" already. I guess this could be done with a separate change if there's a value in it. > > + 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. Fixed with the `test_max_string_one' description. I have also fixed the Ada test case to use `require allow_ada_tests', recently introduced, and updated the copyright years for new files throughout. I'm posting the final version of the patch committed, in a reply to this message. Thank you for your review. Maciej