From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 69633 invoked by alias); 29 Apr 2015 18:39:38 -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 69587 invoked by uid 89); 29 Apr 2015 18:39:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham 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 18:39:36 +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 7D1AD91746; Wed, 29 Apr 2015 18:39:35 +0000 (UTC) Received: from valrhona.uglyboxes.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t3TIdYEk003673 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 29 Apr 2015 14:39:35 -0400 Message-ID: <55412566.2020403@redhat.com> Date: Wed, 29 Apr 2015 19:19:00 -0000 From: Keith Seitz User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Doug Evans CC: gdb-patches Subject: Re: [PATCH 00/18] Implement full completer limiting References: <20150413192235.29172.13097.stgit@valrhona.uglyboxes.com> In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2015-04/txt/msg01083.txt.bz2 On 04/19/2015 08:21 PM, Doug Evans wrote: > I've gone over the entire patch set. A few things I like, but there's > at least one thing I'm concerned about. Replicating the above switch > in each completer: IWBN to avoid such duplication. > We should still be able to remove the global state and fix 17960. The original patch series (necessarily IMO) exposed this memory-management issue, and a global maintainer approved the change. Surely the approving maintainer realized that this code would propagate to other completer functions eventually, no? [I certainly hope the maintainer did not accept that the complete_line hack would be long-lived!] I can certainly change the series to add a function which attempts to hide this detail. For example: /* Returns whether the maximum number of completions has been reached/exceeded. */ int add_completion (struct completer_data *cdata, const char *match) { char *match = xstrdup (name); enum maybe_add_completion_enum add_status; add_status = maybe_add_completion (cdata, match); switch (add_status) { case MAYBE_ADD_COMPLETION_OK: VEC_safe_push (char_ptr, cdata->results, match); break; case MAYBE_ADD_COMPLETION_OK_MAX_REACHED: VEC_safe_push (char_ptr, cdata->results, match); return 1; case MAYBE_ADD_COMPLETION_MAX_REACHED: xfree (match); return 1; case MAYBE_ADD_COMPLETION_DUPLICATE: xfree (match); break; } return 0; } Given that completions (like symbol reading/searching) are extremely time-consuming, one of my design goals was to not introduce anything that might slow down completion, including multiple allocations and copying. Unfortunately with this goal in mind, the use of this proposed function (alone) can only be pushed into four of the fourteen functions dealing with completions. The ten remaining functions would require an additional malloc/copy or yet another API function, e.g., add_completion_1/add_completionA/add_completion_Ex [:-)] to deal with this case. All this to simply forgo checking the return result of maybe_add_completion. I don't see the benefit. Developers already have to deal with memory management in exception handling, which is more complex than simply checking the return result of a function -- especially in (what I would perceive as) cut-n-paste code like this. Or perhaps you had another idea in mind? In any case, let me know what you'd like me to do, and I'll get right at it. Keith