From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 97885 invoked by alias); 29 Apr 2015 10:44:41 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 97876 invoked by uid 89); 29 Apr 2015 10:44:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 29 Apr 2015 10:44:40 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 60BA0A12DC; Wed, 29 Apr 2015 10:44:38 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t3TAiaOG010251; Wed, 29 Apr 2015 06:44:37 -0400 Message-ID: <5540B614.8020104@redhat.com> Date: Wed, 29 Apr 2015 12:37:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Patrick Palka CC: "gdb-patches@sourceware.org" Subject: Re: [PATCH] Fix PR gdb/17820 References: <1430073669-31059-1-git-send-email-patrick@parcs.ath.cx> <553E826E.70300@redhat.com> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-04/txt/msg01064.txt.bz2 On 04/28/2015 02:05 AM, Patrick Palka wrote: > On Mon, Apr 27, 2015 at 2:39 PM, Pedro Alves wrote: >> But now we see that "set history size unlimited" in .gdbinit never >> really worked. Bummer. So this means that users must have kept >> using "set history size 0" instead... > > "set history size 0" (in the .gdbinit) doesn't set history to > unlimited either -- it sets it to 256! So users currently have no > choice but to use "set history size BIGINT" for > approximately-unlimited history. Indeed, looks like that has already been the case... >> So if we change this now, there's no way to have a single >> "set history size FOO" setting that means unlimited with >> both gdb <= 7.9 and the next gdb release. :-/ Oh well. Maybe we should >> just tell users to do "set history size BIGINT" instead? > > But currently, neither "set history size 0" nor "set history size > unlimited" mean unlimited (in the .gdbinit). So users today cannot > set an unlimited history size at all. Changing this now will now at > least make "set history size unlimited". So whether or not we change > this now, there will be no way to have a single "set history size FOO" > setting that means unlimited across current and future GDB versions. > Am I misunderstanding? > >> >> I'd definitely like to make "set history size 0" really >> disable history. >> >> So I think that if goes forward, it'd be good to have a NEWS entry. > > I can think of two things to mention. One is that "set history size > 0" now really disables history. The other is that "set history size > unlimited" now really does not truncate history. Do you have anything > else in mind? That's good. Maybe add the suggestion to use "set history size BIGINT" in .gdbinit, if compatibility with previous releases is desired. >>> PR gdb/17820 >>> * top.c (history_size_setshow_var): Change type to signed. >>> Initialize to -2. Update documentation. >>> (set_readline_history_size): Define. >>> (set_history_size_command): Use it. Remove logic for handling >>> out-of-range sizes. >>> (init_history): Use set_readline_history_size(). Test for a >>> value of -2 instead of 0 when determining whether to set a >>> default history size. >>> (init_main): Decode the argument of the "size" command as a >>> zuinteger_unlimited. >> >> Looks good to me. >> >> Adding a testcase would be ideal, but I'll not make it a requirement. >> I think we should be able to write one making use of GDBFLAGSs. (and >> IWBN to test the GDBHISTSIZE/HISTSIZE environment variables too, which >> we can do with "set env(HISTSIZE)", etc.) > > Do you have in mind a test that creates a dummy .gdbinit file to be > read by GDB? Or is there another way to test this code path? It may be testable with -x or -ix on the command line too, not sure, gdb.base/bp-cmds-execution-x-script.exp is an example, though given "set history size" already behaves different today depending on when it is called, an alternate way to test the issue that happens to use the same path today may change in the future, and we may (re)introducing unnoticed bugs. So I think we should test that path exactly. I was thinking of creating a dir, put a test .gdbinit file there, and point HOME at that dir. We'd just skip the test on remote host testing. Thanks, Pedro Alves