From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7803 invoked by alias); 20 Jul 2011 14:30:44 -0000 Received: (qmail 7794 invoked by uid 22791); 20 Jul 2011 14:30:44 -0000 X-SWARE-Spam-Status: No, hits=-6.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 20 Jul 2011 14:30:29 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p6KEUNET004131 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 20 Jul 2011 10:30:23 -0400 Received: from localhost.localdomain (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p6KEUJUc012988; Wed, 20 Jul 2011 10:30:22 -0400 From: Phil Muldoon To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [rfc] Prompt memory management/cleanups References: <201107201514.46833.pedro@codesourcery.com> Reply-to: pmuldoon@redhat.com X-URL: http://www.redhat.com Date: Wed, 20 Jul 2011 14:37:00 -0000 In-Reply-To: <201107201514.46833.pedro@codesourcery.com> (Pedro Alves's message of "Wed, 20 Jul 2011 15:14:46 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-IsSubscribed: yes 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 X-SW-Source: 2011-07/txt/msg00526.txt.bz2 Pedro Alves writes: > On Wednesday 20 July 2011 14:05:11, Phil Muldoon wrote: >> +set_prompt (const char *s, int level) >> { >> -/* ??rehrauer: I don't know why this fails, since it looks as though >> - assignments to prompt are wrapped in calls to xstrdup... >> - if (prompt != NULL) >> - xfree (prompt); >> - */ >> - PROMPT (0) = xstrdup (s); >> + /* If S is NULL, just free the PROMPT at level LEVEL and set to >> + NULL. */ >> + if (s == NULL) >> + { >> + xfree (PROMPT (level)); >> + PROMPT (level) = NULL; >> + } > > >> + else >> + /* If S == PROMPT then do not free it or set it. If we did >> + that, S (which points to PROMPT), would become garbage. */ >> + if (s != PROMPT (level)) >> + { > > This looks strange, and I suppose complicates the callers' life a bit, > having to know when are they giving up ownership of the string > or not. What would need to change at the callers if we dropped > that s != PROMPT() check? This is a fail-safe for the Python prompt substitution patch. So if they did (the equivalent in Python): s = get_prompt (0) set_prompt (s, 0) Without that check, 'PROMPT (level)' would be freed, but 's' points to that. So you set garbage. get_prompt returns a pointer, not a copy. Internal uses of set_prompt we can just do the checks wherever we want. Basically checking if your pointer to the prompt is in-fact the current prompt. I thought about doing this at the callers, but it just replicated the code everywhere, and in static (IE internal GDB code) that seemed pointless. We could just xstrdup s, then free the prompt anyway, and then set it. But then the s they passed to the function, will still be pointing to xfree'd space; I am not sure if that matters. This seemed the safest way, and not allow prompts to become corrupted. I am completely open to changing it if there is a better way. Cheers, Phil