Re: [rfc] Prompt memory management/cleanups

[Phil Muldoon <pmuldoon at redhat dot com>]

Pedro Alves <pedro@codesourcery.com> 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