[rfc] Prompt memory management/cleanups

[Phil Muldoon <pmuldoon at redhat dot com>]

While working on the Python patch to allow prompt substitution in
Python, I noticed that in some cases prompts were leaking memory.  In
several cases, scenarios like: PROMPT (0) = xstrdup(someprompt), did not
xfree the prompt first.  I decided to use functions calls to replace
access to PROMPT/SUFFIX/PREFIX so that memory management can be
performed centrally.  Attached is a patch.  I'd appreciate comments on
this; the testsuite shows no regressions, but my knowledge of the prompt
area of GDB is about three days old. I'd especially like comments on the
acceptance on NULL in the set_prompt/prefix/suffix functions.  Pop/push
prompt uses different prompt levels and requires that, after use, they
are cleaned.  It normally just does this with xfree.  I'm unsure if my
API for that scenario is preferable.

Cheers

Phil

--

2011-07-20  Phil Muldoon  <pmuldoon@redhat.com>

	* event-top.c (cli_command_loop): Use get_prompt, get_suffix,
	get_prefix.
	(display_gdb_prompt): Likewise.
	(change_annotation_level): Likewise.
	(push_prompt): Likewise.
	(pop_prompt): Likewise.
	(handle_stop_sig): Use get_prompt with a level.
	* top.c (command_loop): Use get_prompt with a level.
	(set_async_annotation_level): Use set_prompt with a level.
	(get_prefix): New function.
	(set_prefix): Ditto.
	(set_suffix): Ditto.
	(get_suffix): Ditto.
	(get_prompt): Accept a level argument.
	(set_prompt): Accept a level argument.  Account for NULL prompts.
	Free old prompts.  Set new_async_prompt if level is 0.
	(init_main): Use set_prompt with a level.  Do not set
	new_async_prompt.
	* top.h: Declare set_suffix, get_suffix, set_prefix, get_prefix.
	Modify set_prompt, get_prompt to account for levels.
	* tui/tui-interp.c (tui_command_loop): Use get_prompt with a
	level.


--

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 72cbfdc..3546ac0 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -190,17 +190,17 @@ cli_command_loop (void)
     {
       int length;
       char *a_prompt;
-      char *gdb_prompt = get_prompt ();
+      char *gdb_prompt = get_prompt (0);
 
       /* Tell readline what the prompt to display is and what function
          it will need to call after a whole line is read.  This also
          displays the first prompt.  */
-      length = strlen (PREFIX (0)) 
-	+ strlen (gdb_prompt) + strlen (SUFFIX (0)) + 1;
+      length = strlen (get_prefix (0))
+	+ strlen (gdb_prompt) + strlen (get_suffix(0)) + 1;
       a_prompt = (char *) alloca (length);
-      strcpy (a_prompt, PREFIX (0));
+      strcpy (a_prompt, get_prefix (0));
       strcat (a_prompt, gdb_prompt);
-      strcat (a_prompt, SUFFIX (0));
+      strcat (a_prompt, get_suffix (0));
       rl_callback_handler_install (a_prompt, input_handler);
     }
   else
@@ -258,7 +258,7 @@ void
 display_gdb_prompt (char *new_prompt)
 {
   int prompt_length = 0;
-  char *gdb_prompt = get_prompt ();
+  char *gdb_prompt = get_prompt (0);
 
   /* Reset the nesting depth used when trace-commands is set.  */
   reset_command_nest_depth ();
@@ -291,19 +291,21 @@ display_gdb_prompt (char *new_prompt)
 
   if (!new_prompt)
     {
       /* Just use the top of the prompt stack.  */
-      prompt_length = strlen (PREFIX (0)) +
-	strlen (SUFFIX (0)) +
+      prompt_length = strlen (get_prefix (0)) +
+	strlen (get_suffix (0)) +
 	strlen (gdb_prompt) + 1;
 
       new_prompt = (char *) alloca (prompt_length);
 
       /* Prefix needs to have new line at end.  */
-      strcpy (new_prompt, PREFIX (0));
+      strcpy (new_prompt, get_prefix (0));
       strcat (new_prompt, gdb_prompt);
       /* Suffix needs to have a new line at end and \032 \032 at
          beginning.  */
-      strcat (new_prompt, SUFFIX (0));
+      strcat (new_prompt, get_suffix (0));
     }
 
   if (async_command_editing_p)
@@ -333,7 +335,7 @@ change_annotation_level (void)
 {
   char *prefix, *suffix;
 
-  if (!PREFIX (0) || !PROMPT (0) || !SUFFIX (0))
+  if (!get_prefix (0) || !get_prompt (0) || !get_suffix (0))
     {
       /* The prompt stack has not been initialized to "", we are
          using gdb w/o the --async switch.  */
@@ -343,7 +345,7 @@ change_annotation_level (void)
 
   if (annotation_level > 1)
     {
-      if (!strcmp (PREFIX (0), "") && !strcmp (SUFFIX (0), ""))
+      if (!strcmp (get_prefix (0), "") && !strcmp (get_suffix (0), ""))
 	{
 	  /* Push a new prompt if the previous annotation_level was not >1.  */
 	  prefix = (char *) alloca (strlen (async_annotation_suffix) + 10);
@@ -361,7 +363,7 @@ change_annotation_level (void)
     }
   else
     {
-      if (strcmp (PREFIX (0), "") && strcmp (SUFFIX (0), ""))
+      if (strcmp (get_prefix (0), "") && strcmp (get_suffix (0), ""))
 	{
 	  /* Pop the top of the stack, we are going back to annotation < 1.  */
 	  pop_prompt ();
@@ -377,17 +379,17 @@ void
 push_prompt (char *prefix, char *prompt, char *suffix)
 {
   the_prompts.top++;
-  PREFIX (0) = xstrdup (prefix);
+  set_prefix (prefix, 0);
 
   /* Note that this function is used by the set annotate 2
      command.  This is why we take care of saving the old prompt
      in case a new one is not specified.  */
   if (prompt)
-    PROMPT (0) = xstrdup (prompt);
+    set_prompt (prompt, 0);
   else
-    PROMPT (0) = xstrdup (PROMPT (-1));
+    set_prompt (get_prompt (-1), 0);
 
-  SUFFIX (0) = xstrdup (suffix);
+  set_suffix (suffix, 0);
 }
 
 /* Pops the top of the prompt stack, and frees the memory allocated
@@ -397,20 +399,17 @@ pop_prompt (void)
 {
   /* If we are not during a 'synchronous' execution command, in which
      case, the top prompt would be empty.  */
-  if (strcmp (PROMPT (0), ""))
+  if (strcmp (get_prompt (0), ""))
     /* This is for the case in which the prompt is set while the
        annotation level is 2.  The top prompt will be changed, but when
        we return to annotation level < 2, we want that new prompt to be
        in effect, until the user does another 'set prompt'.  */
-    if (strcmp (PROMPT (0), PROMPT (-1)))
-      {
-	xfree (PROMPT (-1));
-	PROMPT (-1) = xstrdup (PROMPT (0));
-      }
-
-  xfree (PREFIX (0));
-  xfree (PROMPT (0));
-  xfree (SUFFIX (0));
+    if (strcmp (get_prompt (0), get_prompt (-1)))
+      set_prompt (get_prompt (0), -1);
+
+  set_prefix (NULL, 0);
+  set_prompt (NULL, 0);
+  set_suffix (NULL, 0);
   the_prompts.top--;
 }
 
@@ -955,7 +954,7 @@ handle_stop_sig (int sig)
 static void
 async_stop_sig (gdb_client_data arg)
 {
-  char *prompt = get_prompt ();
+  char *prompt = get_prompt (0);
 
 #if STOP_SIGNAL == SIGTSTP
   signal (SIGTSTP, SIG_DFL);
@@ -1033,7 +1032,7 @@ set_async_annotation_level (char *args, int from_tty,
 void
 set_async_prompt (char *args, int from_tty, struct cmd_list_element *c)
 {
-  PROMPT (0) = xstrdup (new_async_prompt);
+  set_prompt (new_async_prompt, 0);
 }
 
 /* Set things up for readline to be invoked via the alternate
diff --git a/gdb/top.c b/gdb/top.c
index ecb91f2..b0ac474 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -533,7 +533,7 @@ command_loop (void)
   while (instream && !feof (instream))
     {
       if (window_hook && instream == stdin)
-	(*window_hook) (instream, get_prompt ());
+	(*window_hook) (instream, get_prompt (0));
 
       quit_flag = 0;
       if (instream == stdin && stdin_is_tty)
@@ -542,7 +542,7 @@ command_loop (void)
 
       /* Get a command-line.  This calls the readline package.  */
       command = command_line_input (instream == stdin ?
-				    get_prompt () : (char *) NULL,
+				    get_prompt (0) : (char *) NULL,
 				    instream == stdin, "prompt");
       if (command == 0)
 	{
@@ -1124,24 +1124,104 @@ and \"show warranty\" for details.\n");
     }
 }
 
+
+
+/* get_prefix: access method for the GDB prefix string.  */
+
+char *
+get_prefix (int level)
+{
+  return PREFIX (level);
+}
+
+/* set_prefix: set method for the GDB prefix string.  */
+
+void
+set_prefix (const char *s, int level)
+{
+  /* If S is NULL, just free the PREFIX at level LEVEL and set to
+     NULL.  */
+  if (s == NULL)
+    {
+      xfree (PREFIX (level));
+      PREFIX (level) = NULL;
+    }
+  else
+  /* If S == PREFIX then do not free it or set it.  If we did
+     that, S (which points to PREFIX), would become garbage.  */
+    if (s != PREFIX (level))
+      {
+	xfree (PREFIX (level));
+	PREFIX (level) =  xstrdup (s);
+      }
+}
+
+/* get_suffix: access method for the GDB suffix string.  */
+
+char *
+get_suffix (int level)
+{
+  return SUFFIX (level);
+}
+
+/* set_suffix: set method for the GDB suffix string.  */
+
+void
+set_suffix (const char *s, int level)
+{
+  /* If S is NULL, just free the SUFFIX at level LEVEL and set to
+     NULL.  */
+  if (s == NULL)
+    {
+      xfree (SUFFIX (level));
+      SUFFIX (level) = NULL;
+    }
+  else
+    /* If S == SUFFIX then do not free it or set it.  If we did
+       that, S (which points to SUFFIX), would become garbage.  */
+    if (s != SUFFIX (level))
+      {
+	xfree (SUFFIX (level));
+	SUFFIX (level) =  xstrdup (s);
+      }
+}
+
 /* get_prompt: access method for the GDB prompt string.  */
 
 char *
-get_prompt (void)
+get_prompt (int level)
 {
-  return PROMPT (0);
+  return PROMPT (level);
 }
 
 void
-set_prompt (char *s)
+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))
+      {
+	xfree (PROMPT (level));
+	PROMPT (level) =  xstrdup (s);
+
+	if (level == 0)
+	  {
+	  /* Also, free and set new_async_prompt so prompt changes sync up
+	     with set/show prompt.  */
+	    xfree (new_async_prompt);
+	    new_async_prompt = xstrdup (PROMPT (level));
+	  }
+      }
 }
+
 
 
 struct qt_args
@@ -1531,13 +1611,11 @@ init_main (void)
      whatever the DEFAULT_PROMPT is.  */
   the_prompts.top = 0;
   PREFIX (0) = "";
-  PROMPT (0) = xstrdup (DEFAULT_PROMPT);
+  set_prompt (DEFAULT_PROMPT, 0);
   SUFFIX (0) = "";
   /* Set things up for annotation_level > 1, if the user ever decides
      to use it.  */
   async_annotation_suffix = "prompt";
-  /* Set the variable associated with the setshow prompt command.  */
-  new_async_prompt = xstrdup (PROMPT (0));
 
   /* If gdb was started with --annotate=2, this is equivalent to the
      user entering the command 'set annotate 2' at the gdb prompt, so
diff --git a/gdb/top.h b/gdb/top.h
index 4a6e8fb..bdc29c0 100644
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -51,11 +51,27 @@ extern struct cleanup *prepare_execute_command (void);
 
 /* This function returns a pointer to the string that is used
    by gdb for its command prompt.  */
-extern char *get_prompt (void);
+extern char *get_prompt (int);
 
 /* This function copies the specified string into the string that
    is used by gdb for its command prompt.  */
-extern void set_prompt (char *);
+extern void set_prompt (const char *, int);
+
+/* This function returns a pointer to the string that is used
+   by gdb for its command prompt prefix.  */
+extern char *get_prefix (int);
+
+/* This function copies the specified string into the string that
+   is used by gdb for its command prompt prefix.  */
+extern void set_prefix (const char *, int);
+
+/* This function returns a pointer to the string that is used
+   by gdb for its command prompt suffix.  */
+extern char *get_suffix (int);
+
+/* This function copies the specified string into the string that
+   is used by gdb for its command prompt suffix.  */
+extern void set_suffix (const char *, int);
 
 /* From random places.  */
 extern int readnow_symbol_files;
diff --git a/gdb/tui/tui-interp.c b/gdb/tui/tui-interp.c
index d79de2b..5ed692a 100644
--- a/gdb/tui/tui-interp.c
+++ b/gdb/tui/tui-interp.c
@@ -145,7 +145,7 @@ tui_command_loop (void *data)
     {
       int length;
       char *a_prompt;
-      char *gdb_prompt = get_prompt ();
+      char *gdb_prompt = get_prompt (0);
 
       /* Tell readline what the prompt to display is and what function
          it will need to call after a whole line is read. This also