From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 114313 invoked by alias); 22 May 2015 00:42:00 -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 114303 invoked by uid 89); 22 May 2015 00:42:00 -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; Fri, 22 May 2015 00:41:59 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t4M0fupM013938 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 21 May 2015 20:41:56 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t4M0fsXb024225; Thu, 21 May 2015 20:41:55 -0400 Message-ID: <555E7B52.6050100@redhat.com> Date: Fri, 22 May 2015 00:42: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] Tweak the handling of $HISTSIZE edge cases [PR gdb/16999] References: <1432248648-7402-1-git-send-email-patrick@parcs.ath.cx> <555E6B60.8040802@redhat.com> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-05/txt/msg00592.txt.bz2 On 05/22/2015 01:26 AM, Patrick Palka wrote: > On Thu, May 21, 2015 at 7:33 PM, Pedro Alves wrote: >> If I'm reading correctly, this treats HISTSIZE=" " as "disable history". >> Is that intended? > > It's not really intended. The motivation was to make sure that an > obvious typo in HISTSIZE (e.g. HISTSIZE="1000-") will not truncate the > history size, but HISTSIZE=" " is not really a typo. But IMO adding a > more intelligent typo heuristic (one to replace *endptr != '\0') is > not worth it -- it's just a history file after all. I haven't gone back to recheck what bash does, but, I can see that happening in scripts, like: if whatever; then mysize=1000 fi HISTSIZE="$mysize " HISTFILESIZE="$mysize" and then mysize ends up unset. > > But that reminds me that the strings " 10" and "10 " should not be > considered non-numeric. That could easily be achieved via a couple of > calls to isspace(). Exactly, I was thinking of those too, but I didn't want to call out what the behavior should be. :-) > >> >> Also, a nit: I find it a bit odd to see strlen to check empty string >> in one case, and != '\0' in another, instead of: >> >> if (*tmpenv == '\0' >> || var < 0 >> || *endptr != '\0') >> >>> + history_size_setshow_var = -1; >>> + else >>> + history_size_setshow_var = var; >>> } >>> /* If the init file hasn't set a size yet, pick the default. */ >>> else if (history_size_setshow_var == -2) > > Well semantically endptr is less of a string and more of a pointer to > a char within a string -- at least that's how I view it. But I will > change the first condition to check for '\0'. Ah. Good point. I'm fine either way then. > > On a related note, I wonder whether it is a good idea for GDB to look > at HISTSIZE at all. As the buildbots and you have shown, some distros > export HISTSIZE by default and by doing so it renders useless GDB's > internal "history size" setting (as far as .gdbinit is concerned). I > think people expect HISTSIZE to only affect shells, not e.g. readline > applications. (Otherwise, I would expect the readline library to > already extract the default history size from HISTSIZE or from another > environment variable, something it currently has no support for.) So > I wonder whether it would be better to stop reading HISTSIZE, to > instead read GDBHISTSIZE or something. Yeah, I'm inclined to agree. Thanks, Pedro Alves