[PATCH 3/3] gdb/tui: Use cleanups to free string copies.

[PATCH 3/3] gdb/tui: Use cleanups to free string copies.

[Andrew Burgess]

In parse_scrolling_args it is possible for a string copy to leak if an
error occurs.  Switching to using a cleanup fixes this leak.

In tui_set_win_height the string can't be leaked, but switching to using
a cleanup guards against the possibility that a leak could be introduced
in the future (by adding an error somewhere in the call stack).

gdb/ChangeLog:

	* tui/tui-win.c (tui_set_win_height): Use a cleanup to free the
	string copy.
	(parse_scrolling_args): Likewise.
---
 gdb/ChangeLog     | 6 ++++++
 gdb/tui/tui-win.c | 9 ++++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 646bc5b..e995f01 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2015-07-06  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* tui/tui-win.c (tui_set_win_height): Use a cleanup to free the
+	string copy.
+	(parse_scrolling_args): Likewise.
+
+2015-07-06  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* tui/tui-win.c (focus_completer): Don't duplicate the tui window
 	names in this function.
 
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 947608a..cdf322b 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -1141,7 +1141,9 @@ tui_set_win_height (char *arg, int from_tty)
       char *wname = (char *) NULL;
       int new_height, i;
       struct tui_win_info *win_info;
+      struct cleanup *old_chain;
 
+      old_chain = make_cleanup (xfree, buf);
       wname = buf_ptr;
       buf_ptr = strchr (buf_ptr, ' ');
       if (buf_ptr != (char *) NULL)
@@ -1204,8 +1206,7 @@ The window name specified must be valid and visible.\n"));
       else
 	printf_filtered (WIN_HEIGHT_USAGE);
 
-      if (buf != (char *) NULL)
-	xfree (buf);
+      do_cleanups (old_chain);
     }
   else
     printf_filtered (WIN_HEIGHT_USAGE);
@@ -1636,9 +1637,11 @@ parse_scrolling_args (char *arg,
   if (arg != (char *) NULL)
     {
       char *buf, *buf_ptr;
+      struct cleanup *old_chain;
 
       /* Process the number of lines to scroll.  */
       buf = buf_ptr = xstrdup (arg);
+      old_chain = make_cleanup (xfree, buf);
       if (isdigit (*buf_ptr))
 	{
 	  char *num_str;
@@ -1686,6 +1689,6 @@ The window name specified must be valid and visible.\n"));
 	  else if (*win_to_scroll == TUI_CMD_WIN)
 	    *win_to_scroll = (tui_source_windows ())->list[0];
 	}
-      xfree (buf);
+      do_cleanups (old_chain);
     }
 }
-- 
2.4.0

[PATCH 0/3] convert tui window names to lower case

[Andrew Burgess]

This patch series converts the tui window names to lower case.  The
window names are already lower case in the documentation, and the
recently added command completer for 'focus' reports window names as
lower case.

The window names aren't actally shown in that many places, so this
change shouldn't be that big of a upset (I hope), plus all the
commands that take a window name already accepts both upper and lower
case.

Patch #1 does the conversion to lower case.

Patch #2 is a follow-on clean up.

Patch #3 is a small related bug fix, I've left it in this chain as it
         would other wise conflict with patch #1.

Thanks,
Andrew

--

Andrew Burgess (3):
  gdb: Convert TUI windows names to lower case.
  gdb/tui: Define tui window names once.
  gdb/tui: Use cleanups to free string copies.

 gdb/ChangeLog      | 21 +++++++++++++++++++++
 gdb/tui/tui-data.h |  8 ++++----
 gdb/tui/tui-win.c  | 46 ++++++++++++++++------------------------------
 3 files changed, 41 insertions(+), 34 deletions(-)

-- 
2.4.0

[PATCH 2/3] gdb/tui: Define tui window names once.

[Andrew Burgess]

Don't duplicate the window names inside the completion function.
Instead make use of the existing defines, and the tui_win_name function
to obtain the window names.

gdb/ChangeLog:

	* tui/tui-win.c (focus_completer): Don't duplicate the tui window
	names in this function.
---
 gdb/ChangeLog     |  5 +++++
 gdb/tui/tui-win.c | 27 +++++----------------------
 2 files changed, 10 insertions(+), 22 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2e889e9..646bc5b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2015-07-06  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* tui/tui-win.c (focus_completer): Don't duplicate the tui window
+	names in this function.
+
+2015-07-06  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* tui/tui-data.h (SRC_NAME): Convert to lower case.
 	(CMD_NAME): Likewise.
 	(DATA_NAME): Likewise.
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 215a7f5..947608a 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -374,26 +374,9 @@ focus_completer (struct cmd_list_element *ignore,
 	  || !tui_win_list[win_type]->generic.is_visible)
 	continue;
 
-      switch (win_type)
-	{
-	case SRC_WIN:
-	  completion_name = "src";
-	  break;
-	case DISASSEM_WIN:
-	  completion_name = "asm";
-	  break;
-	case DATA_WIN:
-	  completion_name = "regs";
-	  break;
-	case CMD_WIN:
-	  completion_name = "cmd";
-	  break;
-	default:
-	  break;
-	}
-
-      if (completion_name != NULL)
-	VEC_safe_push (const_char_ptr, completion_name_vec, completion_name);
+      completion_name = tui_win_name (&tui_win_list [win_type]->generic);
+      gdb_assert (completion_name != NULL);
+      VEC_safe_push (const_char_ptr, completion_name_vec, completion_name);
     }
 
   /* If no windows are considered visible then the TUI has not yet been
@@ -402,8 +385,8 @@ focus_completer (struct cmd_list_element *ignore,
      default layout to SRC_COMMAND.  */
   if (VEC_length (const_char_ptr, completion_name_vec) == 0)
     {
-      VEC_safe_push (const_char_ptr, completion_name_vec, "src");
-      VEC_safe_push (const_char_ptr, completion_name_vec, "cmd");
+      VEC_safe_push (const_char_ptr, completion_name_vec, SRC_NAME);
+      VEC_safe_push (const_char_ptr, completion_name_vec, CMD_NAME);
     }
 
   VEC_safe_push (const_char_ptr, completion_name_vec, "next");
-- 
2.4.0

[PATCH 1/3] gdb: Convert TUI windows names to lower case.

[Andrew Burgess]

This commit converts the window names for the TUI windows to lower case.
The windows names are already lower case in the documentation, and are
shown as lower case in some of the command completion results.

Given that all the commands that take a window name currently accept
upper or lower case, this commit just changes the window names to lower
case in the remaining places they are displayed by gdb.

gdb/ChangeLog:

	* tui/tui-data.h (SRC_NAME): Convert to lower case.
	(CMD_NAME): Likewise.
	(DATA_NAME): Likewise.
	(DISASSEM_NAME): Likewise.
	* tui/tui-win.c (tui_set_focus): Window names are now lower case.
	(tui_set_win_height): Likewise.
	(parse_scrolling_args): Likewise.
---
 gdb/ChangeLog      | 10 ++++++++++
 gdb/tui/tui-data.h |  8 ++++----
 gdb/tui/tui-win.c  | 10 +++++-----
 3 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3bdb099..2e889e9 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2015-07-06  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* tui/tui-data.h (SRC_NAME): Convert to lower case.
+	(CMD_NAME): Likewise.
+	(DATA_NAME): Likewise.
+	(DISASSEM_NAME): Likewise.
+	* tui/tui-win.c (tui_set_focus): Window names are now lower case.
+	(tui_set_win_height): Likewise.
+	(parse_scrolling_args): Likewise.
+
 2015-07-08  Patrick Palka  <patrick@parcs.ath.cx>
 
 	* defs.h (deprecated_register_changed_hook): Remove prototype.
diff --git a/gdb/tui/tui-data.h b/gdb/tui/tui-data.h
index 1b9d832..0e7e31e 100644
--- a/gdb/tui/tui-data.h
+++ b/gdb/tui/tui-data.h
@@ -60,10 +60,10 @@ struct tui_gen_win_info
 #define NO_REGS_STRING          "[ Register Values Unavailable ]"
 #define NO_DATA_STRING          "[ No Data Values Displayed ]"
 #define MAX_CONTENT_COUNT       100
-#define SRC_NAME                "SRC"
-#define CMD_NAME                "CMD"
-#define DATA_NAME               "REGS"
-#define DISASSEM_NAME           "ASM"
+#define SRC_NAME                "src"
+#define CMD_NAME                "cmd"
+#define DATA_NAME               "regs"
+#define DISASSEM_NAME           "asm"
 #define TUI_NULL_STR            ""
 #define DEFAULT_HISTORY_COUNT	25
 #define BOX_WINDOW              TRUE
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 629d54d..215a7f5 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -1036,11 +1036,11 @@ tui_set_focus (char *arg, int from_tty)
       struct tui_win_info *win_info = (struct tui_win_info *) NULL;
 
       for (i = 0; (i < strlen (buf_ptr)); i++)
-	buf_ptr[i] = toupper (arg[i]);
+	buf_ptr[i] = tolower (arg[i]);
 
-      if (subset_compare (buf_ptr, "NEXT"))
+      if (subset_compare (buf_ptr, "next"))
 	win_info = tui_next_win (tui_win_with_focus ());
-      else if (subset_compare (buf_ptr, "PREV"))
+      else if (subset_compare (buf_ptr, "prev"))
 	win_info = tui_prev_win (tui_win_with_focus ());
       else
 	win_info = tui_partial_win_by_name (buf_ptr);
@@ -1167,7 +1167,7 @@ tui_set_win_height (char *arg, int from_tty)
 
 	  /* Validate the window name.  */
 	  for (i = 0; i < strlen (wname); i++)
-	    wname[i] = toupper (wname[i]);
+	    wname[i] = tolower (wname[i]);
 	  win_info = tui_partial_win_by_name (wname);
 
 	  if (win_info == (struct tui_win_info *) NULL
@@ -1689,7 +1689,7 @@ parse_scrolling_args (char *arg,
 
 	      /* Validate the window name.  */
 	      for (i = 0; i < strlen (wname); i++)
-		wname[i] = toupper (wname[i]);
+		wname[i] = tolower (wname[i]);
 	    }
 	  else
 	    wname = "?";
-- 
2.4.0

Re: [PATCH 0/3] convert tui window names to lower case

[Patrick Palka]

On Wed, Jul 8, 2015 at 4:37 PM, Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
> This patch series converts the tui window names to lower case.  The
> window names are already lower case in the documentation, and the
> recently added command completer for 'focus' reports window names as
> lower case.
>
> The window names aren't actally shown in that many places, so this
> change shouldn't be that big of a upset (I hope), plus all the
> commands that take a window name already accepts both upper and lower
> case.

Cool, I thought about doing something like this too, after I added the
"focus" completer.

What about the occurrences of "SRC", "CMD", etc in tui-layout.c?
These do not seem to be changed in this series but it seems like they
should.

Re: [PATCH 0/3] convert tui window names to lower case

[Pedro Alves]

On 07/08/2015 09:51 PM, Patrick Palka wrote:
> On Wed, Jul 8, 2015 at 4:37 PM, Andrew Burgess
> <andrew.burgess@embecosm.com> wrote:
>> This patch series converts the tui window names to lower case.  The
>> window names are already lower case in the documentation, and the
>> recently added command completer for 'focus' reports window names as
>> lower case.
>>
>> The window names aren't actally shown in that many places, so this
>> change shouldn't be that big of a upset (I hope), plus all the
>> commands that take a window name already accepts both upper and lower
>> case.
> 
> Cool, I thought about doing something like this too, after I added the
> "focus" completer.
> 
> What about the occurrences of "SRC", "CMD", etc in tui-layout.c?
> These do not seem to be changed in this series but it seems like they
> should.

Nice cleanup.  This is once that issue is resolved and Patrick is
happy with it.  :-)

Thanks,
Pedro Alves

Re: [PATCH 0/3] convert tui window names to lower case

[Patrick Palka]

On Wed, Jul 8, 2015 at 4:51 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> On Wed, Jul 8, 2015 at 4:37 PM, Andrew Burgess
> <andrew.burgess@embecosm.com> wrote:
>> This patch series converts the tui window names to lower case.  The
>> window names are already lower case in the documentation, and the
>> recently added command completer for 'focus' reports window names as
>> lower case.
>>
>> The window names aren't actally shown in that many places, so this
>> change shouldn't be that big of a upset (I hope), plus all the
>> commands that take a window name already accepts both upper and lower
>> case.
>
> Cool, I thought about doing something like this too, after I added the
> "focus" completer.
>
> What about the occurrences of "SRC", "CMD", etc in tui-layout.c?
> These do not seem to be changed in this series but it seems like they
> should.

On second thought, never mind this concern.  The occurrences of "SRC",
"CMD", etc in tui-layout.c are referring to layout names, not window
names, so they should not necessarily be replaced by the window-name
macros.  Besides, it can always be done in a separate patch.  So FWIW
I am happy with this series :)