From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from eggs.gnu.org (eggs.gnu.org [209.51.188.92]) by sourceware.org (Postfix) with ESMTPS id 9067A3857C7D for ; Wed, 26 Jan 2022 16:53:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9067A3857C7D Received: from [2001:470:142:3::e] (port=36396 helo=fencepost.gnu.org) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nClXb-0006eR-6s; Wed, 26 Jan 2022 11:53:15 -0500 Received: from ip5f5a8abe.dynamic.kabel-deutschland.de ([95.90.138.190]:60053 helo=[192.168.111.41]) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1nClXY-0001jz-23; Wed, 26 Jan 2022 11:53:02 -0500 Message-ID: <6d92837c-ffe1-dbf3-bf61-15ecbcc140ea@gnu.org> Date: Wed, 26 Jan 2022 17:52:56 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Content-Language: en-US To: "Maciej W. Rozycki" Cc: Andrew Burgess , gdb-patches@sourceware.org References: <111e4bb6-d781-ff80-f64b-125ad665f502@web.de> <20220106093322.GD828155@redhat.com> From: Simon Sobisch Subject: Re: [PATCH v2] gdb: split array and string limiting options In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, SPF_HELO_PASS, SPF_PASS, TXREP 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: Wed, 26 Jan 2022 16:53:19 -0000 Am 26.01.2022 um 16:48 schrieb Maciej W. Rozycki: > Hi Simon, > > Your message wasn't directly cc-ed to me and I missed it previously in > the flood of mailing-list traffic. Sorry about this. Thanks for still coming back :-) > Please note that I have posted v3 before your message already. I see, so some of my questions are possibly also not up-tp-date- >>>> In my opinion there should be only 1 new test for the new setting and >>>> the other should be left unchanged while still working, because then >>>> existing usages will still work, too. > > Agreed. That's something we can work with - let's check the implementations and options below for that. > >>>> To do so the new parameter cannot be directly used as it is in the patch >>>> but must be fowarded to a new function or duplicated. >>>> >>>> As an example this would mean instead of : >>>> >>>> --- src.orig/gdb/c-valprint.c >>>> +++ src/gdb/c-valprint.c >>>> @@ -271,7 +271,7 @@ c_value_print_array (struct value *val, >>>> >>>> for (temp_len = 0; >>>> (temp_len < len >>>> - && temp_len < options->print_max >>>> + && temp_len < options->print_max_chars >>>> && extract_unsigned_integer (valaddr + temp_len * eltlen, >>>> eltlen, byte _order) != 0); >>>> ++temp_len) >>>> >>>> have the following >>>> >>>> >>>> + const int print_max_chars = (options->print_max_chars != -1) ? >>>> options->print_max_chars : options->print_max; >>>> for (temp_len = 0; >>>> (temp_len < len >>>> - && temp_len < options->print_max >>>> + && temp_len < print_max_chars >>>> && extract_unsigned_integer (valaddr + temp_len * eltlen, >>>> eltlen, byte_order) != 0); >>>> ++temp_len) >>> Proposal 1 - Should work, shouldn't it? > If you want to keep backwards compatibility, then no particular value for > `set print elements' can be used to drive `set print characters' I'm > afraid. Someone may have used it somewhere. What do you mean with that? If the default is whatever print elements is it should be compatible, shouldn't it? Please elaborate. Also: is that option "still on the table"? > What I can offer as possible alternative solutions are: > > * As you proposed, but instead driven by an entirely separate knob such as > `set print characters-enable '; if "off", which would also be > the default, then keeping the current semantics of `set print elements'. Proposal 2 - That's an interesting idea. We could also say that in a later GDB version the default will be changed to on. The best thing here: it is easily possible to have a different default for print characters (possibly much higher than print elements) with still keeping backward compatibility. > * Leave `set print elements' alone and instead introduce a replacement > say `set print array-elements' command for array elements in addition to > `set print characters' already have proposed. The latter two commands > would adjust the respective settings each while the former command would > set them both, for compatibility. It's not clear to me what `show print > elements' would have to report however, maybe the `print array-elements' > setting. Proposal 3 - also an interesting idea, a clean split. I tend to this one. > > * Have `set print elements' adjust both limits till `set print characters' > or `show print characters' have been used, at which point further > requests would only adjust the array element limit. This wouldn't pose > a problem with `show print elements', but the semantics would be > complicated. Proposal 4 - has "complicated in" - let's drop it. > Thoughts? > > Maciej Thanks again for working on this, Simon