PATCH: Fix TUI null pointer dereference

PATCH: Fix TUI null pointer dereference

[Andrew STUBBS]

[-- Attachment #1: Type: text/plain, Size: 360 bytes --]

Hi,

The attached patch fixes a NULL pointer dereference starting the TUI 
when the TERM environment variable is not set. This is a particular 
problem on windows where that variable is not usually set.

2005-10-17  Andrew Stubbs  <andrew.stubbs@st.com>

	* tui/tui-command.c (tui_dispatch_ctrl_char): Test output of
	getenv() before using it.

Andrew Stubbs


[-- Attachment #2: tui-NULL-pointer.patch --]
[-- Type: text/plain, Size: 624 bytes --]

Index: src/gdb/tui/tui-command.c
===================================================================
--- src.orig/gdb/tui/tui-command.c	2004-02-16 21:05:09.000000000 +0000
+++ src/gdb/tui/tui-command.c	2005-10-17 14:33:06.000000000 +0100
@@ -70,7 +70,7 @@ tui_dispatch_ctrl_char (unsigned int ch)
       term = (char *) getenv ("TERM");
       for (i = 0; (term && term[i]); i++)
 	term[i] = toupper (term[i]);
-      if ((strcmp (term, "XTERM") == 0) && key_is_start_sequence (ch))
+      if (term && (strcmp (term, "XTERM") == 0) && key_is_start_sequence (ch))
 	{
 	  unsigned int page_ch = 0;
 	  unsigned int tmp_char;

Re: PATCH: Fix TUI null pointer dereference

[Jim Blandy]

Andrew STUBBS <andrew.stubbs@st.com> writes:
> Index: src/gdb/tui/tui-command.c
> ===================================================================
> --- src.orig/gdb/tui/tui-command.c	2004-02-16 21:05:09.000000000 +0000
> +++ src/gdb/tui/tui-command.c	2005-10-17 14:33:06.000000000 +0100
> @@ -70,7 +70,7 @@ tui_dispatch_ctrl_char (unsigned int ch)
>        term = (char *) getenv ("TERM");
>        for (i = 0; (term && term[i]); i++)
>  	term[i] = toupper (term[i]);
> -      if ((strcmp (term, "XTERM") == 0) && key_is_start_sequence (ch))
> +      if (term && (strcmp (term, "XTERM") == 0) && key_is_start_sequence (ch))
>  	{
>  	  unsigned int page_ch = 0;
>  	  unsigned int tmp_char;

How about wrapping the upcasing loop and the 'if' after it all in one
big 'if (term), starting right after the 'getenv ("TERM")', and then
simplifying the conditional in the upcasing loop?

Re: PATCH: Fix TUI null pointer dereference

[Andrew STUBBS]

[-- Attachment #1: Type: text/plain, Size: 1088 bytes --]

Jim Blandy wrote:
> Andrew STUBBS <andrew.stubbs@st.com> writes:
> 
>>Index: src/gdb/tui/tui-command.c
>>===================================================================
>>--- src.orig/gdb/tui/tui-command.c	2004-02-16 21:05:09.000000000 +0000
>>+++ src/gdb/tui/tui-command.c	2005-10-17 14:33:06.000000000 +0100
>>@@ -70,7 +70,7 @@ tui_dispatch_ctrl_char (unsigned int ch)
>>       term = (char *) getenv ("TERM");
>>       for (i = 0; (term && term[i]); i++)
>> 	term[i] = toupper (term[i]);
>>-      if ((strcmp (term, "XTERM") == 0) && key_is_start_sequence (ch))
>>+      if (term && (strcmp (term, "XTERM") == 0) && key_is_start_sequence (ch))
>> 	{
>> 	  unsigned int page_ch = 0;
>> 	  unsigned int tmp_char;
> 
> 
> How about wrapping the upcasing loop and the 'if' after it all in one
> big 'if (term), starting right after the 'getenv ("TERM")', and then
> simplifying the conditional in the upcasing loop?
> 

How about the attached? Diff has made it look rather confusing, but I 
assure you the only thing that has changed is the indenting and the 
extra if.

Andrew Stubbs

[-- Attachment #2: tui-NULL-pointer.patch --]
[-- Type: text/plain, Size: 1859 bytes --]

2005-10-24  Andrew Stubbs  <andrew.stubbs@st.com>

	* tui/tui-command.c (tui_dispatch_ctrl_char): Test output of
	getenv() before using it.


Index: src/gdb/tui/tui-command.c
===================================================================
--- src.orig/gdb/tui/tui-command.c	2005-10-24 13:58:38.000000000 +0100
+++ src/gdb/tui/tui-command.c	2005-10-24 14:11:02.000000000 +0100
@@ -67,34 +67,36 @@ tui_dispatch_ctrl_char (unsigned int ch)
          ** by keypad as a single char, so we must handle them here.
          ** Seems like a bug in the curses library?
        */
-      term = (char *) getenv ("TERM");
-      for (i = 0; (term && term[i]); i++)
-	term[i] = toupper (term[i]);
-      if ((strcmp (term, "XTERM") == 0) && key_is_start_sequence (ch))
+      if (term = (char *) getenv ("TERM"))
 	{
-	  unsigned int page_ch = 0;
-	  unsigned int tmp_char;
-
-	  tmp_char = 0;
-	  while (!key_is_end_sequence (tmp_char))
+	  for (i = 0; term[i]; i++)
+	    term[i] = toupper (term[i]);
+	  if ((strcmp (term, "XTERM") == 0) && key_is_start_sequence (ch))
 	    {
-	      tmp_char = (int) wgetch (w);
-	      if (tmp_char == ERR)
-		{
-		  return ch;
-		}
-	      if (!tmp_char)
-		break;
-	      if (tmp_char == 53)
-		page_ch = KEY_PPAGE;
-	      else if (tmp_char == 54)
-		page_ch = KEY_NPAGE;
-	      else
+	      unsigned int page_ch = 0;
+	      unsigned int tmp_char;
+
+	      tmp_char = 0;
+	      while (!key_is_end_sequence (tmp_char))
 		{
-		  return 0;
+		  tmp_char = (int) wgetch (w);
+		  if (tmp_char == ERR)
+		    {
+		      return ch;
+		    }
+		  if (!tmp_char)
+		    break;
+		  if (tmp_char == 53)
+		    page_ch = KEY_PPAGE;
+		  else if (tmp_char == 54)
+		    page_ch = KEY_NPAGE;
+		  else
+		    {
+		      return 0;
+		    }
 		}
+	      ch_copy = page_ch;
 	    }
-	  ch_copy = page_ch;
 	}
 
       switch (ch_copy)

Re: PATCH: Fix TUI null pointer dereference

[Andrew STUBBS]

[-- Attachment #1: Type: text/plain, Size: 392 bytes --]

Jim Blandy wrote:
> We generally avoid using assignments directly as conditions.  Just
> hoist the assignment out and just say 'if (term)'.  With that change,
> feel free to commit.

OK. Here is an updated patch.

I do not have write permission/ability. If I could get that set up that 
would be great, but till then somebody else wil have to submit it for me 
please.

Thanks

Andrew Stubbs

[-- Attachment #2: tui-NULL-pointer.patch --]
[-- Type: text/plain, Size: 1762 bytes --]

2005-10-24  Andrew Stubbs  <andrew.stubbs@st.com>

	* tui/tui-command.c (tui_dispatch_ctrl_char): Test output of
	getenv() before using it.


Index: src/gdb/tui/tui-command.c
===================================================================
--- src.orig/gdb/tui/tui-command.c	2005-10-24 13:58:38.000000000 +0100
+++ src/gdb/tui/tui-command.c	2005-10-24 18:56:04.000000000 +0100
@@ -68,33 +68,36 @@ tui_dispatch_ctrl_char (unsigned int ch)
          ** Seems like a bug in the curses library?
        */
       term = (char *) getenv ("TERM");
-      for (i = 0; (term && term[i]); i++)
-	term[i] = toupper (term[i]);
-      if ((strcmp (term, "XTERM") == 0) && key_is_start_sequence (ch))
+      if (term)
 	{
-	  unsigned int page_ch = 0;
-	  unsigned int tmp_char;
-
-	  tmp_char = 0;
-	  while (!key_is_end_sequence (tmp_char))
+	  for (i = 0; term[i]; i++)
+	    term[i] = toupper (term[i]);
+	  if ((strcmp (term, "XTERM") == 0) && key_is_start_sequence (ch))
 	    {
-	      tmp_char = (int) wgetch (w);
-	      if (tmp_char == ERR)
-		{
-		  return ch;
-		}
-	      if (!tmp_char)
-		break;
-	      if (tmp_char == 53)
-		page_ch = KEY_PPAGE;
-	      else if (tmp_char == 54)
-		page_ch = KEY_NPAGE;
-	      else
+	      unsigned int page_ch = 0;
+	      unsigned int tmp_char;
+
+	      tmp_char = 0;
+	      while (!key_is_end_sequence (tmp_char))
 		{
-		  return 0;
+		  tmp_char = (int) wgetch (w);
+		  if (tmp_char == ERR)
+		    {
+		      return ch;
+		    }
+		  if (!tmp_char)
+		    break;
+		  if (tmp_char == 53)
+		    page_ch = KEY_PPAGE;
+		  else if (tmp_char == 54)
+		    page_ch = KEY_NPAGE;
+		  else
+		    {
+		      return 0;
+		    }
 		}
+	      ch_copy = page_ch;
 	    }
-	  ch_copy = page_ch;
 	}
 
       switch (ch_copy)

Re: PATCH: Fix TUI null pointer dereference

[Daniel Jacobowitz]

On Mon, Oct 24, 2005 at 06:58:56PM +0100, Andrew STUBBS wrote:
> Jim Blandy wrote:
> >We generally avoid using assignments directly as conditions.  Just
> >hoist the assignment out and just say 'if (term)'.  With that change,
> >feel free to commit.
> 
> OK. Here is an updated patch.
> 
> I do not have write permission/ability. If I could get that set up that 
> would be great, but till then somebody else wil have to submit it for me 
> please.

You've got lots of pending patches (that I was going to review this
weekend but ran out of time... it will get done this week).  So, let's
get you write access.  Go to sourceware.org, look for the link that
says "handy dandy little form", and list me as approver.

It looks like ST has corporate assignments in place, so there won't be
any trouble on that front.


-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: PATCH: Fix TUI null pointer dereference

[Andrew STUBBS]

Daniel Jacobowitz wrote:
> You've got lots of pending patches (that I was going to review this
> weekend but ran out of time... it will get done this week).  So, let's
> get you write access.  Go to sourceware.org, look for the link that
> says "handy dandy little form", and list me as approver.

Thanks, I have submitted the form.

I also have quite a few patched queued at this end for your future 
entertainment. I won't post them until I have finished tweaking and 
testing and got the current lot out of the way.

Andrew Stubbs

Re: PATCH: Fix TUI null pointer dereference

[Andrew STUBBS]

Andrew Stubbs wrote:
> Jim Blandy wrote:
> 
>> We generally avoid using assignments directly as conditions.  Just
>> hoist the assignment out and just say 'if (term)'.  With that change,
>> feel free to commit.
> 
> 
> OK. Here is an updated patch.
> 
> I do not have write permission/ability. If I could get that set up that 
> would be great, but till then somebody else wil have to submit it for me 
> please.

I have now commited this myself!

Thanks to all who made it possible. Now on to the next ...

Andrew