Re: [patch] [python] Prompt substitution

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> +@findex gdb.get_current_prompt
Phil> +@defun get_current_prompt
Phil> +Return the current @value{GDBN} prompt as a string.
Phil> +@end defun

Is this different from gdb.parameter('prompt')?

Phil> +@findex gdb.set_current_prompt
Phil> +@defun set_current_prompt @r{[}new_prompt@r{]}
Phil> +Sets the @value{GDBN} prompt to @var{new_prompt}.  @var{new_prompt}
Phil> +must be a string.  This method has no return value.
Phil> +@end defun

I'm on the fence about this.  It would probably be better to have a
general facility for setting parameters.

Phil> +@defop Operation {@value{GDBN}} prompt_hook [current_prompt]
Phil> +If @var{prompt_hook} exists, and is not set to @code{None},

... really if it is callable.

Phil> +The parameter @code{current_prompt} contains the current @value{GDBN} 
Phil> +prompt.

If the parameter is not optional, then it should not be surrounded in []
in the @defop.

Phil> +      observer_notify_before_prompt (pre_gdb_prompt);
Phil> +      post_gdb_prompt = get_prompt ();
Phil> +
Phil> +      /* If the observer changed the prompt, use that prompt overriding
Phil> +	 any new_prompt that was passed to this function.  */
Phil> +      if (strcmp (pre_gdb_prompt, post_gdb_prompt) != 0)

Suppose an observer sets the prompt.
Won't this free the memory pointed to by pre_gdb_prompt?
Thus making this strcmp access invalid memory?

Phil> +	  if (current_gdb_prompt == NULL)
Phil> +	    {

When can this happen?

Phil> +	      current_prompt =  PyString_FromString (current_gdb_prompt);

Extra space after the '='.

Phil> +	  if ((result != Py_None) && (! PyString_Check (result)))

Too many parens.

Phil> +  const char *prompt = get_prompt ();
Phil> +  if (prompt)
Phil> +    return PyString_FromString (prompt);

Likewise about whether this can happen.

Phil> +  { "flush", (PyCFunction)gdbpy_flush, METH_VARARGS | METH_KEYWORDS,
Phil> +    "Flush gdb's filtered stdout stream." },

This is already in the code and shouldn't be in this patch.

Phil> +      PROMPT (0) =  xstrdup (s);

Extra space.

Tom