PATCH: Problem union comparision in TUI

PATCH: Problem union comparision in TUI

[Andrew STUBBS]

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

Hi,

I have observed this problem in the sh-elf configuration of GDB. It is 
not visible in the i686-linux configuration because it uses the 
following union in a different way.

/* Structure describing source line or line address */
union tui_line_or_address
{
   int line_no;
   CORE_ADDR addr;
};


The problem is that the union is always compared using 'addr'. In the 
case where it was set using 'line_no', as it is in sh-elf, half the 
union contains garbage. This appears to be killing the comparison in 
tui_set_is_exec_point_at() in tui-winsource.c.

The result is that the current line is never highlighted as it should 
be. I assume the reason it works on i686-linux is that that host/target 
uses the addr field rather than the line_no field.

The attached patch provides one way to fix the problem. I could not 
think of any way to fix the comparison because there is no way I can see 
to know which mode it is using, so I have changed the type of line_no in 
the union to match the type of addr. This does not seem to be the Right 
Thing to do with a union (because it might as well be one variable), but 
I can't see any other down side.

Andrew Stubbs

[-- Attachment #2: tui-union-compare.patch --]
[-- Type: text/plain, Size: 429 bytes --]

Index: src/gdb/tui/tui-data.h
===================================================================
--- src.orig/gdb/tui/tui-data.h	2004-03-13 14:14:01.000000000 +0000
+++ src/gdb/tui/tui-data.h	2005-10-17 14:21:23.000000000 +0100
@@ -149,7 +149,7 @@ enum tui_register_display_type
 /* Structure describing source line or line address */
 union tui_line_or_address
 {
-  int line_no;
+  CORE_ADDR line_no;
   CORE_ADDR addr;
 };
 

Re: PATCH: Problem union comparision in TUI

[Andrew STUBBS]

Appoogies, I forgot to attach a Changelog entry.

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

	* tui/tui-data.h (tui_line_or_address): Change type of line_no
	from int to CORE_ADDR (to match addr).

Re: PATCH: Problem union comparision in TUI

[Eli Zaretskii]

> Date: Mon, 17 Oct 2005 15:51:21 +0100
> From: Andrew STUBBS <andrew.stubbs@st.com>
> 
> --- src.orig/gdb/tui/tui-data.h	2004-03-13 14:14:01.000000000 +0000
> +++ src/gdb/tui/tui-data.h	2005-10-17 14:21:23.000000000 +0100
> @@ -149,7 +149,7 @@ enum tui_register_display_type
>  /* Structure describing source line or line address */
>  union tui_line_or_address
>  {
> -  int line_no;
> +  CORE_ADDR line_no;
>    CORE_ADDR addr;
>  };

IMHO, a better way of fixing this would be to change
tui_line_or_address to be a struct as follows:

  /* Structure describing source line or line address */
  struct tui_line_or_address
  {
     unsigned char what
     union {
             int line_no;
	     CORE_ADDR addr;
	   } u;
  };

and then fill the `what' member as appropriate and use the right part
of the union in the comparison.

Re: PATCH: Problem union comparision in TUI

[Andrew STUBBS]

Eli Zaretskii wrote:
> IMHO, a better way of fixing this would be to change
> tui_line_or_address to be a struct as follows:
> 
>   /* Structure describing source line or line address */
>   struct tui_line_or_address
>   {
>      unsigned char what
>      union {
>              int line_no;
> 	     CORE_ADDR addr;
> 	   } u;
>   };
> 
> and then fill the `what' member as appropriate and use the right part
> of the union in the comparison.
> 

Yes, that might be true, but that would be a much larger change and I 
would be much less confident of having done it right, plus it will add 
some complexity.

A quick grep says there are 51 references to 'line_or_addr', but there 
are about 15 places other names are declared for this thing so all the 
uses might take a little more tracking down. I suppose changing the name 
of the members and seeing what breaks will be the best way.

Is there any reason for using a union here? It's not like one value is 
float and the other int - both are ints and the fact that you can't tell 
which you are using shows nobody actually uses the distinction (unless I 
have missed something). The union doesn't save any space, nor does it 
make the code any more efficient.

We could just use:

CORE_ADDR line_or_address;

and leave it at that.

Anyway, I am happy to modify the patch to whatever is suggested.

Thanks

Andrew Stubbs

Re: PATCH: Problem union comparision in TUI

[Daniel Jacobowitz]

On Wed, Oct 19, 2005 at 10:48:53AM +0100, Andrew STUBBS wrote:
> Is there any reason for using a union here? It's not like one value is 
> float and the other int - both are ints and the fact that you can't tell 
> which you are using shows nobody actually uses the distinction (unless I 
> have missed something). The union doesn't save any space, nor does it 
> make the code any more efficient.
> 
> We could just use:
> 
> CORE_ADDR line_or_address;
> 
> and leave it at that.

I'd rather that.

It seems possible to figure out which one it's using - always file line
numbers for source displays and addresses for assembly displays.

          if ((win == TUI_SRC_WIN
               && bp->source_file
               && (strcmp (src->filename, bp->source_file) == 0)
               && bp->line_number == line->line_or_addr.line_no)
              || (win == TUI_DISASM_WIN
                  && bp->loc->address == line->line_or_addr.addr))

But why bother...

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: PATCH: Problem union comparision in TUI

[Andrew STUBBS]

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

Daniel Jacobowitz wrote:
> On Wed, Oct 19, 2005 at 10:48:53AM +0100, Andrew STUBBS wrote:
> 
>>Is there any reason for using a union here? It's not like one value is 
>>float and the other int - both are ints and the fact that you can't tell 
>>which you are using shows nobody actually uses the distinction (unless I 
>>have missed something). The union doesn't save any space, nor does it 
>>make the code any more efficient.
>>
>>We could just use:
>>
>>CORE_ADDR line_or_address;
>>
>>and leave it at that.
> 
> 
> I'd rather that.

OK, how about the attached patch?

Andrew

[-- Attachment #2: tui-union-compare.patch --]
[-- Type: text/plain, Size: 19491 bytes --]

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

	* tui/tui-data.h (tui_line_or_address): Convert from union to a
	single type.
	(struct tui_source_element, struct tui_source_info): Update.
	* tui/tui-data.c, tui/tui-disasm.c, tui/tui-source.c: Update.
	* tui/tui-stack.c, tui/tui-win.c, tui-winsource.c: Update.
	* tui/tui-winsource.h, tui/tui-layout.c, tui/tui-source.h: Update.


Index: src/gdb/tui/tui-data.h
===================================================================
--- src.orig/gdb/tui/tui-data.h	2005-10-19 15:35:49.000000000 +0100
+++ src/gdb/tui/tui-data.h	2005-10-19 15:36:05.000000000 +0100
@@ -146,12 +146,8 @@ enum tui_register_display_type
   TUI_GENERAL_AND_SPECIAL_REGS
 };
 
-/* Structure describing source line or line address */
-union tui_line_or_address
-{
-  int line_no;
-  CORE_ADDR addr;
-};
+/* Type describing source line or line address */
+typedef CORE_ADDR tui_line_or_address;
 
 /* Current Layout definition */
 struct tui_layout_def
@@ -166,7 +162,7 @@ struct tui_layout_def
 struct tui_source_element
 {
   char *line;
-  union tui_line_or_address line_or_addr;
+  tui_line_or_address line_or_addr;
   int is_exec_point;
   int has_break;
 };
@@ -259,7 +255,7 @@ struct tui_source_info
   /* Execution information window.  */
   struct tui_gen_win_info *execution_info;
   int horizontal_offset;	/* used for horizontal scroll */
-  union tui_line_or_address start_line_or_addr;
+  tui_line_or_address start_line_or_addr;
   char* filename;
 };
 
Index: src/gdb/tui/tui-data.c
===================================================================
--- src.orig/gdb/tui/tui-data.c	2005-10-19 15:35:49.000000000 +0100
+++ src/gdb/tui/tui-data.c	2005-10-19 15:36:05.000000000 +0100
@@ -207,7 +207,7 @@ tui_clear_win_detail (struct tui_win_inf
 	{
 	case SRC_WIN:
 	case DISASSEM_WIN:
-	  win_info->detail.source_info.start_line_or_addr.addr = 0;
+	  win_info->detail.source_info.start_line_or_addr = 0;
 	  win_info->detail.source_info.horizontal_offset = 0;
 	  break;
 	case CMD_WIN:
@@ -486,7 +486,7 @@ init_content_element (struct tui_win_ele
     case SRC_WIN:
     case DISASSEM_WIN:
       element->which_element.source.line = (char *) NULL;
-      element->which_element.source.line_or_addr.line_no = 0;
+      element->which_element.source.line_or_addr = 0;
       element->which_element.source.is_exec_point = FALSE;
       element->which_element.source.has_break = FALSE;
       break;
@@ -537,7 +537,7 @@ init_win_info (struct tui_win_info * win
       win_info->detail.source_info.execution_info = (struct tui_gen_win_info *) NULL;
       win_info->detail.source_info.has_locator = FALSE;
       win_info->detail.source_info.horizontal_offset = 0;
-      win_info->detail.source_info.start_line_or_addr.addr = 0;
+      win_info->detail.source_info.start_line_or_addr = 0;
       win_info->detail.source_info.filename = 0;
       break;
     case DATA_WIN:
Index: src/gdb/tui/tui-disasm.c
===================================================================
--- src.orig/gdb/tui/tui-disasm.c	2005-10-19 15:35:49.000000000 +0100
+++ src/gdb/tui/tui-disasm.c	2005-10-19 15:38:07.000000000 +0100
@@ -188,7 +188,7 @@ tui_set_disassem_content (CORE_ADDR pc)
   if (ret != TUI_SUCCESS)
     return ret;
 
-  TUI_DISASM_WIN->detail.source_info.start_line_or_addr.addr = pc;
+  TUI_DISASM_WIN->detail.source_info.start_line_or_addr = pc;
   cur_pc = (CORE_ADDR)
     (((struct tui_win_element *) locator->content[0])->which_element.locator.addr);
 
@@ -249,7 +249,7 @@ tui_set_disassem_content (CORE_ADDR pc)
       else
         src->line[0] = '\0';
 
-      src->line_or_addr.addr = asm_lines[i].addr;
+      src->line_or_addr = asm_lines[i].addr;
       src->is_exec_point = asm_lines[i].addr == cur_pc;
 
       /* See whether there is a breakpoint installed.  */
@@ -270,9 +270,9 @@ tui_show_disassem (CORE_ADDR start_addr)
 {
   struct symtab *s = find_pc_symtab (start_addr);
   struct tui_win_info * win_with_focus = tui_win_with_focus ();
-  union tui_line_or_address val;
+  tui_line_or_address val;
 
-  val.addr = start_addr;
+  val = start_addr;
   tui_add_win_to_layout (DISASSEM_WIN);
   tui_update_source_window (TUI_DISASM_WIN, s, val, FALSE);
   /*
@@ -295,14 +295,14 @@ tui_show_disassem_and_update_source (COR
   tui_show_disassem (start_addr);
   if (tui_current_layout () == SRC_DISASSEM_COMMAND)
     {
-      union tui_line_or_address val;
+      tui_line_or_address val;
 
       /*
          ** Update what is in the source window if it is displayed too,
          ** note that it follows what is in the disassembly window and visa-versa
        */
       sal = find_pc_line (start_addr, 0);
-      val.line_no = sal.line;
+      val = sal.line;
       tui_update_source_window (TUI_SRC_WIN, sal.symtab, val, TRUE);
       if (sal.symtab)
 	{
@@ -376,7 +376,7 @@ tui_vertical_disassem_scroll (enum tui_s
       CORE_ADDR pc;
       tui_win_content content;
       struct symtab *s;
-      union tui_line_or_address val;
+      tui_line_or_address val;
       int max_lines, dir;
       struct symtab_and_line cursal = get_current_source_symtab_and_line ();
 
@@ -388,10 +388,10 @@ tui_vertical_disassem_scroll (enum tui_s
 
       /* account for hilite */
       max_lines = TUI_DISASM_WIN->generic.height - 2;
-      pc = content[0]->which_element.source.line_or_addr.addr;
+      pc = content[0]->which_element.source.line_or_addr;
       dir = (scroll_direction == FORWARD_SCROLL) ? max_lines : - max_lines;
 
-      val.addr = tui_find_disassembly_address (pc, dir);
+      val = tui_find_disassembly_address (pc, dir);
       tui_update_source_window_as_is (TUI_DISASM_WIN, s, val, FALSE);
     }
 }
Index: src/gdb/tui/tui-source.c
===================================================================
--- src.orig/gdb/tui/tui-source.c	2005-10-19 15:35:49.000000000 +0100
+++ src/gdb/tui/tui-source.c	2005-10-19 16:02:59.000000000 +0100
@@ -106,7 +106,7 @@ tui_set_source_content (struct symtab *s
 		  stream = fdopen (desc, FOPEN_RT);
 		  clearerr (stream);
 		  cur_line = 0;
-		  cur_line_no = src->start_line_or_addr.line_no = line_no;
+		  cur_line_no = src->start_line_or_addr = line_no;
 		  if (offset > 0)
 		    src_line = (char *) xmalloc (
 					   (threshold + 1) * sizeof (char));
@@ -137,8 +137,7 @@ tui_set_source_content (struct symtab *s
 
 		      /* Set whether element is the execution point and
 		         whether there is a break point on it.  */
-		      element->which_element.source.line_or_addr.line_no =
-			cur_line_no;
+		      element->which_element.source.line_or_addr = cur_line_no;
 		      element->which_element.source.is_exec_point =
 			(strcmp (((struct tui_win_element *)
 			locator->content[0])->which_element.locator.file_name,
@@ -247,7 +246,7 @@ tui_set_source_content_nil (struct tui_w
 
       struct tui_win_element * element =
       (struct tui_win_element *) win_info->generic.content[curr_line];
-      element->which_element.source.line_or_addr.line_no = 0;
+      element->which_element.source.line_or_addr = 0;
       element->which_element.source.is_exec_point = FALSE;
       element->which_element.source.has_break = FALSE;
 
@@ -295,7 +294,7 @@ tui_set_source_content_nil (struct tui_w
 /* Function to display source in the source window.  This function
    initializes the horizontal scroll to 0.  */
 void
-tui_show_symtab_source (struct symtab *s, union tui_line_or_address line, int noerror)
+tui_show_symtab_source (struct symtab *s, tui_line_or_address line, int noerror)
 {
   TUI_SRC_WIN->detail.source_info.horizontal_offset = 0;
   tui_update_source_window_as_is (TUI_SRC_WIN, s, line, noerror);
@@ -320,7 +319,7 @@ tui_vertical_source_scroll (enum tui_scr
 {
   if (TUI_SRC_WIN->generic.content != NULL)
     {
-      union tui_line_or_address l;
+      tui_line_or_address l;
       struct symtab *s;
       tui_win_content content = (tui_win_content) TUI_SRC_WIN->generic.content;
       struct symtab_and_line cursal = get_current_source_symtab_and_line ();
@@ -332,21 +331,19 @@ tui_vertical_source_scroll (enum tui_scr
 
       if (scroll_direction == FORWARD_SCROLL)
 	{
-	  l.line_no = content[0]->which_element.source.line_or_addr.line_no +
-	    num_to_scroll;
-	  if (l.line_no > s->nlines)
+	  l = content[0]->which_element.source.line_or_addr + num_to_scroll;
+	  if (l > s->nlines)
 	    /*line = s->nlines - win_info->generic.content_size + 1; */
 	    /*elz: fix for dts 23398 */
-	    l.line_no = content[0]->which_element.source.line_or_addr.line_no;
+	    l = content[0]->which_element.source.line_or_addr;
 	}
       else
 	{
-	  l.line_no = content[0]->which_element.source.line_or_addr.line_no -
-	    num_to_scroll;
-	  if (l.line_no <= 0)
-	    l.line_no = 1;
+	  l = content[0]->which_element.source.line_or_addr - num_to_scroll;
+	  if (l <= 0)
+	    l = 1;
 	}
 
-      print_source_lines (s, l.line_no, l.line_no + 1, 0);
+      print_source_lines (s, l, l + 1, 0);
     }
 }
Index: src/gdb/tui/tui-stack.c
===================================================================
--- src.orig/gdb/tui/tui-stack.c	2005-10-19 15:35:49.000000000 +0100
+++ src/gdb/tui/tui-stack.c	2005-10-19 16:03:56.000000000 +0100
@@ -364,14 +364,14 @@ tui_show_frame_info (struct frame_info *
 
 	  if (win_info == TUI_SRC_WIN)
 	    {
-	      union tui_line_or_address l;
-	      l.line_no = start_line;
+	      tui_line_or_address l;
+	      l = start_line;
 	      if (!(source_already_displayed
 		    && tui_line_is_displayed (item->locator.line_no, win_info, TRUE)))
 		tui_update_source_window (win_info, sal.symtab, l, TRUE);
 	      else
 		{
-		  l.line_no = item->locator.line_no;
+		  l = item->locator.line_no;
 		  tui_set_is_exec_point_at (l, win_info);
 		}
 	    }
@@ -379,13 +379,13 @@ tui_show_frame_info (struct frame_info *
 	    {
 	      if (win_info == TUI_DISASM_WIN)
 		{
-		  union tui_line_or_address a;
-		  a.addr = low;
+		  tui_line_or_address a;
+		  a = low;
 		  if (!tui_addr_is_displayed (item->locator.addr, win_info, TRUE))
 		    tui_update_source_window (win_info, sal.symtab, a, TRUE);
 		  else
 		    {
-		      a.addr = item->locator.addr;
+		      a = item->locator.addr;
 		      tui_set_is_exec_point_at (a, win_info);
 		    }
 		}
Index: src/gdb/tui/tui-win.c
===================================================================
--- src.orig/gdb/tui/tui-win.c	2005-10-19 15:35:49.000000000 +0100
+++ src/gdb/tui/tui-win.c	2005-10-19 16:04:50.000000000 +0100
@@ -1322,31 +1322,29 @@ make_visible_with_new_height (struct tui
       tui_make_visible (win_info->detail.source_info.execution_info);
       if (win_info->generic.content != NULL)
 	{
-	  union tui_line_or_address line_or_addr;
+	  tui_line_or_address line_or_addr;
 	  struct symtab_and_line cursal
 	    = get_current_source_symtab_and_line ();
 
 	  if (win_info->generic.type == SRC_WIN)
-	    line_or_addr.line_no =
-	      win_info->detail.source_info.start_line_or_addr.line_no;
+	    line_or_addr = win_info->detail.source_info.start_line_or_addr;
 	  else
-	    line_or_addr.addr =
-	      win_info->detail.source_info.start_line_or_addr.addr;
+	    line_or_addr = win_info->detail.source_info.start_line_or_addr;
 	  tui_free_win_content (&win_info->generic);
 	  tui_update_source_window (win_info, cursal.symtab, line_or_addr, TRUE);
 	}
       else if (deprecated_selected_frame != (struct frame_info *) NULL)
 	{
-	  union tui_line_or_address line;
+	  tui_line_or_address line;
 	  struct symtab_and_line cursal = get_current_source_symtab_and_line ();
 
 
 	  s = find_pc_symtab (get_frame_pc (deprecated_selected_frame));
 	  if (win_info->generic.type == SRC_WIN)
-	    line.line_no = cursal.line;
+	    line = cursal.line;
 	  else
 	    {
-	      find_line_pc (s, cursal.line, &line.addr);
+	      find_line_pc (s, cursal.line, &line);
 	    }
 	  tui_update_source_window (win_info, s, line, TRUE);
 	}
Index: src/gdb/tui/tui-winsource.c
===================================================================
--- src.orig/gdb/tui/tui-winsource.c	2005-10-19 15:35:49.000000000 +0100
+++ src/gdb/tui/tui-winsource.c	2005-10-19 15:44:22.000000000 +0100
@@ -71,7 +71,7 @@ tui_display_main (void)
    initializes the horizontal scroll to 0.  */
 void
 tui_update_source_window (struct tui_win_info * win_info, struct symtab *s,
-			  union tui_line_or_address line_or_addr, int noerror)
+			  tui_line_or_address line_or_addr, int noerror)
 {
   win_info->detail.source_info.horizontal_offset = 0;
   tui_update_source_window_as_is (win_info, s, line_or_addr, noerror);
@@ -84,14 +84,14 @@ tui_update_source_window (struct tui_win
    shows the source as specified by the horizontal offset.  */
 void
 tui_update_source_window_as_is (struct tui_win_info * win_info, struct symtab *s,
-				union tui_line_or_address line_or_addr, int noerror)
+				tui_line_or_address line_or_addr, int noerror)
 {
   enum tui_status ret;
 
   if (win_info->generic.type == SRC_WIN)
-    ret = tui_set_source_content (s, line_or_addr.line_no, noerror);
+    ret = tui_set_source_content (s, line_or_addr, noerror);
   else
-    ret = tui_set_disassem_content (line_or_addr.addr);
+    ret = tui_set_disassem_content (line_or_addr);
 
   if (ret == TUI_FAILURE)
     {
@@ -107,8 +107,7 @@ tui_update_source_window_as_is (struct t
 	{
 	  struct symtab_and_line sal;
 	  
-	  sal.line = line_or_addr.line_no +
-	    (win_info->generic.content_size - 2);
+	  sal.line = line_or_addr + (win_info->generic.content_size - 2);
 	  sal.symtab = s;
 	  set_current_source_symtab_and_line (&sal);
 	  /*
@@ -134,7 +133,7 @@ tui_update_source_windows_with_addr (COR
   if (addr != 0)
     {
       struct symtab_and_line sal;
-      union tui_line_or_address l;
+      tui_line_or_address l;
       
       switch (tui_current_layout ())
 	{
@@ -147,7 +146,7 @@ tui_update_source_windows_with_addr (COR
 	  break;
 	default:
 	  sal = find_pc_line (addr, 0);
-	  l.line_no = sal.line;
+	  l = sal.line;
 	  tui_show_symtab_source (sal.symtab, l, FALSE);
 	  break;
 	}
@@ -172,7 +171,7 @@ void
 tui_update_source_windows_with_line (struct symtab *s, int line)
 {
   CORE_ADDR pc;
-  union tui_line_or_address l;
+  tui_line_or_address l;
   
   switch (tui_current_layout ())
     {
@@ -182,7 +181,7 @@ tui_update_source_windows_with_line (str
       tui_update_source_windows_with_addr (pc);
       break;
     default:
-      l.line_no = line;
+      l = line;
       tui_show_symtab_source (s, l, FALSE);
       if (tui_current_layout () == SRC_DISASSEM_COMMAND)
 	{
@@ -336,7 +335,7 @@ tui_horizontal_source_scroll (struct tui
 
 /* Set or clear the has_break flag in the line whose line is line_no.  */
 void
-tui_set_is_exec_point_at (union tui_line_or_address l, struct tui_win_info * win_info)
+tui_set_is_exec_point_at (tui_line_or_address l, struct tui_win_info * win_info)
 {
   int changed = 0;
   int i;
@@ -347,7 +346,7 @@ tui_set_is_exec_point_at (union tui_line
     {
       int new_state;
 
-      if (content[i]->which_element.source.line_or_addr.addr == l.addr)
+      if (content[i]->which_element.source.line_or_addr == l)
         new_state = TRUE;
       else
 	new_state = FALSE;
@@ -417,9 +416,9 @@ tui_update_breakpoint_info (struct tui_w
           if ((win == TUI_SRC_WIN
                && bp->source_file
                && (strcmp (src->filename, bp->source_file) == 0)
-               && bp->line_number == line->line_or_addr.line_no)
+               && bp->line_number == line->line_or_addr)
               || (win == TUI_DISASM_WIN
-                  && bp->loc->address == line->line_or_addr.addr))
+                  && bp->loc->address == line->line_or_addr))
             {
               if (bp->enable_state == bp_disabled)
                 mode |= TUI_BP_DISABLED;
@@ -614,7 +613,7 @@ tui_line_is_displayed (int line, struct 
   while (i < win_info->generic.content_size - threshold && !is_displayed)
     {
       is_displayed = (((struct tui_win_element *)
-		      win_info->generic.content[i])->which_element.source.line_or_addr.line_no
+		      win_info->generic.content[i])->which_element.source.line_or_addr
 		     == (int) line);
       i++;
     }
@@ -640,7 +639,7 @@ tui_addr_is_displayed (CORE_ADDR addr, s
   while (i < win_info->generic.content_size - threshold && !is_displayed)
     {
       is_displayed = (((struct tui_win_element *)
-		      win_info->generic.content[i])->which_element.source.line_or_addr.addr
+		      win_info->generic.content[i])->which_element.source.line_or_addr
 		     == addr);
       i++;
     }
Index: src/gdb/tui/tui-winsource.h
===================================================================
--- src.orig/gdb/tui/tui-winsource.h	2005-10-19 15:35:49.000000000 +0100
+++ src/gdb/tui/tui-winsource.h	2005-10-19 15:36:05.000000000 +0100
@@ -43,10 +43,10 @@ extern int tui_update_breakpoint_info (s
 /* Function to display the "main" routine.  */
 extern void tui_display_main (void);
 extern void tui_update_source_window (struct tui_win_info *, struct symtab *,
-				      union tui_line_or_address, int);
+				      tui_line_or_address, int);
 extern void tui_update_source_window_as_is (struct tui_win_info *,
 					    struct symtab *,
-					    union tui_line_or_address, int);
+					    tui_line_or_address, int);
 extern void tui_update_source_windows_with_addr (CORE_ADDR);
 extern void tui_update_source_windows_with_line (struct symtab *, int);
 extern void tui_clear_source_content (struct tui_win_info *, int);
@@ -60,7 +60,7 @@ extern void tui_erase_exec_info_content 
 extern void tui_clear_exec_info_content (struct tui_win_info *);
 extern void tui_update_exec_info (struct tui_win_info *);
 
-extern void tui_set_is_exec_point_at (union tui_line_or_address,
+extern void tui_set_is_exec_point_at (tui_line_or_address,
 				      struct tui_win_info *);
 extern enum tui_status tui_alloc_source_buffer (struct tui_win_info *);
 extern int tui_line_is_displayed (int, struct tui_win_info *, int);
Index: src/gdb/tui/tui-layout.c
===================================================================
--- src.orig/gdb/tui/tui-layout.c	2005-02-15 15:49:27.000000000 +0000
+++ src/gdb/tui/tui-layout.c	2005-10-19 15:39:17.000000000 +0100
@@ -519,14 +519,14 @@ extract_display_start_addr (void)
     case SRC_COMMAND:
     case SRC_DATA_COMMAND:
       find_line_pc (cursal.symtab,
-		    TUI_SRC_WIN->detail.source_info.start_line_or_addr.line_no,
+		    TUI_SRC_WIN->detail.source_info.start_line_or_addr,
 		    &pc);
       addr = pc;
       break;
     case DISASSEM_COMMAND:
     case SRC_DISASSEM_COMMAND:
     case DISASSEM_DATA_COMMAND:
-      addr = TUI_DISASM_WIN->detail.source_info.start_line_or_addr.addr;
+      addr = TUI_DISASM_WIN->detail.source_info.start_line_or_addr;
       break;
     default:
       addr = 0;
Index: src/gdb/tui/tui-source.h
===================================================================
--- src.orig/gdb/tui/tui-source.h	2004-02-07 01:40:25.000000000 +0000
+++ src/gdb/tui/tui-source.h	2005-10-19 15:40:11.000000000 +0100
@@ -33,7 +33,7 @@ struct tui_win_info;
 extern void tui_set_source_content_nil (struct tui_win_info *, char *);
 
 extern enum tui_status tui_set_source_content (struct symtab *, int, int);
-extern void tui_show_symtab_source (struct symtab *, union tui_line_or_address, int);
+extern void tui_show_symtab_source (struct symtab *, tui_line_or_address, int);
 extern int tui_source_is_displayed (char *);
 extern void tui_vertical_source_scroll (enum tui_scroll_direction, int);
 

Re: PATCH: Problem union comparision in TUI

[Eli Zaretskii]

> Date: Wed, 19 Oct 2005 10:48:53 +0100
> From: Andrew STUBBS <andrew.stubbs@st.com>
> Cc: gdb-patches@sources.redhat.com
> 
> Is there any reason for using a union here?

The reason is that the code should be clean and self-explanatory.
Using the same variable for storing two utterly different objects is
IMHO The Mother Of Unclean Code.

Re: PATCH: Problem union comparision in TUI

[Daniel Jacobowitz]

On Wed, Oct 19, 2005 at 10:03:14PM +0200, Eli Zaretskii wrote:
> > Date: Wed, 19 Oct 2005 10:48:53 +0100
> > From: Andrew STUBBS <andrew.stubbs@st.com>
> > Cc: gdb-patches@sources.redhat.com
> > 
> > Is there any reason for using a union here?
> 
> The reason is that the code should be clean and self-explanatory.
> Using the same variable for storing two utterly different objects is
> IMHO The Mother Of Unclean Code.

Is an untagged union any clearer?

We've already established (via the bug report) that some of the time,
the code has no idea which one is in use when comparing them.  They're
used for relative line ordering within a particular window; if it's a
source window, the lines are sorted by line number, and if it's a
disassembly window, they're sorted by code address.  So in both cases
it's a "line number"; that's why I favor using a single variable for
them, although I'm open to alternative suggestions.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: PATCH: Problem union comparision in TUI

[Mark Kettenis]

> Date: Wed, 19 Oct 2005 16:07:51 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Wed, Oct 19, 2005 at 10:03:14PM +0200, Eli Zaretskii wrote:
> > > Date: Wed, 19 Oct 2005 10:48:53 +0100
> > > From: Andrew STUBBS <andrew.stubbs@st.com>
> > > Cc: gdb-patches@sources.redhat.com
> > > 
> > > Is there any reason for using a union here?
> > 
> > The reason is that the code should be clean and self-explanatory.
> > Using the same variable for storing two utterly different objects is
> > IMHO The Mother Of Unclean Code.
> 
> Is an untagged union any clearer?
> 
> We've already established (via the bug report) that some of the time,
> the code has no idea which one is in use when comparing them.  They're
> used for relative line ordering within a particular window; if it's a
> source window, the lines are sorted by line number, and if it's a
> disassembly window, they're sorted by code address.  So in both cases
> it's a "line number"; that's why I favor using a single variable for
> them, although I'm open to alternative suggestions.

But encoding line numbers in CORE_ADDR is probably a bad idea.  In
many parts of GDB the signedness of the integer type matters.  I've
found this out the hard way, when trying to fix some of the warnings
generated by GCC 4.  I think there is no other option than going over
the code, analyzing what it does and analyzing it accordingly.

Mark

Re: PATCH: Problem union comparision in TUI

[Eli Zaretskii]

> Date: Wed, 19 Oct 2005 16:07:51 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Andrew STUBBS <andrew.stubbs@st.com>, gdb-patches@sources.redhat.com
> 
> > The reason is that the code should be clean and self-explanatory.
> > Using the same variable for storing two utterly different objects is
> > IMHO The Mother Of Unclean Code.
> 
> Is an untagged union any clearer?

No, it isn't.  I suggested to modify the data structure so that the
union is tagged.  Did you see that message?

> We've already established (via the bug report) that some of the time,
> the code has no idea which one is in use when comparing them.  They're
> used for relative line ordering within a particular window; if it's a
> source window, the lines are sorted by line number, and if it's a
> disassembly window, they're sorted by code address.  So in both cases
> it's a "line number"; that's why I favor using a single variable for
> them, although I'm open to alternative suggestions.

Yes, I've read the code before I replied, so I know all that already.
Having read the code, I'm not sure that addresses are used only for
disassembly windows and line numbers only for source windows.  We
could have more bugs; that's why I think cleaning the code is
important.

I think it shouldn't be too hard to make the change I suggested, since
most of it boils down to mechanically adding either the line or
address tag whenever the respective member of the union is assigned a
value.

Re: PATCH: Problem union comparision in TUI

[Andrew STUBBS]

Eli Zaretskii wrote:
>>Date: Wed, 19 Oct 2005 16:07:51 -0400
>>From: Daniel Jacobowitz <drow@false.org>
 >>
>>We've already established (via the bug report) that some of the time,
>>the code has no idea which one is in use when comparing them.  They're
>>used for relative line ordering within a particular window; if it's a
>>source window, the lines are sorted by line number, and if it's a
>>disassembly window, they're sorted by code address.  So in both cases
>>it's a "line number"; that's why I favor using a single variable for
>>them, although I'm open to alternative suggestions.
> 
> 
> Yes, I've read the code before I replied, so I know all that already.
> Having read the code, I'm not sure that addresses are used only for
> disassembly windows and line numbers only for source windows.  We
> could have more bugs; that's why I think cleaning the code is
> important.

Indeed, it isn't as simple as addresses for assembly and lines for 
sources. That's what the original problem was - i686-pc-linux-gnu native 
uses one and sh-elf cross (also running on i686-pc-linux-gnu) uses the 
other while both are supposedly running the same simple program .

> I think it shouldn't be too hard to make the change I suggested, since
> most of it boils down to mechanically adding either the line or
> address tag whenever the respective member of the union is assigned a
> value.

Yeah, it shouldn't be too hard. It just seems redundant.

Andrew Stubbs

Re: PATCH: Problem union comparision in TUI

[Andrew STUBBS]

Andrew Stubbs wrote:
> Eli Zaretskii wrote:
>> Yes, I've read the code before I replied, so I know all that already.
>> Having read the code, I'm not sure that addresses are used only for
>> disassembly windows and line numbers only for source windows.  We
>> could have more bugs; that's why I think cleaning the code is
>> important.
> 
> 
> Indeed, it isn't as simple as addresses for assembly and lines for 
> sources. That's what the original problem was - i686-pc-linux-gnu native 
> uses one and sh-elf cross (also running on i686-pc-linux-gnu) uses the 
> other while both are supposedly running the same simple program .

Apologies, please ignore this. I have just done some experiments to try 
to understand this better.

It would appear that both are using line numbers for the source window. 
The difference is that, for reasons unknown, sh-elf has a 64 bit 
CORE_ADDR, but i686-pc-linux-gnu has a 32 bit CORE_ADDR. line_no remains 
32 bit on both platforms. Hence I only see the problem on targets with 
mismatched types, and then only when random data manages to find its way 
into the unused half of the union.

Therefore, an alternative fix for this problem would be to figure out 
which window we are in for the problem comparison. I an not sure whether 
that would be a better solution or not.

Sorry for the confusion.

Andrew Stubbs

Re: PATCH: Problem union comparision in TUI

[Daniel Jacobowitz]

On Thu, Oct 20, 2005 at 05:18:14PM +0100, Andrew STUBBS wrote:
> Therefore, an alternative fix for this problem would be to figure out 
> which window we are in for the problem comparison. I an not sure whether 
> that would be a better solution or not.

You _should_ already be able to do this: almost all places already
check.  I recommend taking a closer look at just the ones which don't
to see if you can find the window context.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: PATCH: Problem union comparision in TUI

[Eli Zaretskii]

> Date: Thu, 20 Oct 2005 17:18:14 +0100
> From: Andrew STUBBS <andrew.stubbs@st.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, gdb-patches@sources.redhat.com
> 
> Therefore, an alternative fix for this problem would be to figure out 
> which window we are in for the problem comparison. I an not sure whether 
> that would be a better solution or not.

I still think that adding a tag to the union is the cleanest solution.

Re: PATCH: Problem union comparision in TUI

[Andrew STUBBS]

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

Eli Zaretskii wrote:
> I still think that adding a tag to the union is the cleanest solution.

I have attached a patch implementing this. I have also attached a 
slightly improved version of the one I posted before.

I am not entirely happy with the tagged union approach. There are a 
number of places where it just assumes that the union must be doing the 
right thing here. The only other possibility would be to assert. It 
does, however, solve the problem at hand.

Does either of these grab your fancy?

Andrew Stubbs

[-- Attachment #2: tui-union-compare.patch --]
[-- Type: text/plain, Size: 19419 bytes --]

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

	* tui/tui-data.h (tui_line_or_address): Convert from union to a
	single type.
	(struct tui_source_element, struct tui_source_info): Update.
	* tui/tui-data.c, tui/tui-disasm.c, tui/tui-source.c: Update.
	* tui/tui-stack.c, tui/tui-win.c, tui-winsource.c: Update.
	* tui/tui-winsource.h, tui/tui-layout.c, tui/tui-source.h: Update.


Index: src/gdb/tui/tui-data.h
===================================================================
--- src.orig/gdb/tui/tui-data.h	2005-10-20 18:16:44.000000000 +0100
+++ src/gdb/tui/tui-data.h	2005-10-21 15:31:23.000000000 +0100
@@ -146,12 +146,8 @@ enum tui_register_display_type
   TUI_GENERAL_AND_SPECIAL_REGS
 };
 
-/* Structure describing source line or line address */
-union tui_line_or_address
-{
-  int line_no;
-  CORE_ADDR addr;
-};
+/* Type describing source line or line address */
+typedef CORE_ADDR tui_line_or_address;
 
 /* Current Layout definition */
 struct tui_layout_def
@@ -166,7 +162,7 @@ struct tui_layout_def
 struct tui_source_element
 {
   char *line;
-  union tui_line_or_address line_or_addr;
+  tui_line_or_address line_or_addr;
   int is_exec_point;
   int has_break;
 };
@@ -259,7 +255,7 @@ struct tui_source_info
   /* Execution information window.  */
   struct tui_gen_win_info *execution_info;
   int horizontal_offset;	/* used for horizontal scroll */
-  union tui_line_or_address start_line_or_addr;
+  tui_line_or_address start_line_or_addr;
   char* filename;
 };
 
Index: src/gdb/tui/tui-data.c
===================================================================
--- src.orig/gdb/tui/tui-data.c	2005-10-20 18:16:44.000000000 +0100
+++ src/gdb/tui/tui-data.c	2005-10-21 15:31:23.000000000 +0100
@@ -207,7 +207,7 @@ tui_clear_win_detail (struct tui_win_inf
 	{
 	case SRC_WIN:
 	case DISASSEM_WIN:
-	  win_info->detail.source_info.start_line_or_addr.addr = 0;
+	  win_info->detail.source_info.start_line_or_addr = 0;
 	  win_info->detail.source_info.horizontal_offset = 0;
 	  break;
 	case CMD_WIN:
@@ -486,7 +486,7 @@ init_content_element (struct tui_win_ele
     case SRC_WIN:
     case DISASSEM_WIN:
       element->which_element.source.line = (char *) NULL;
-      element->which_element.source.line_or_addr.line_no = 0;
+      element->which_element.source.line_or_addr = 0;
       element->which_element.source.is_exec_point = FALSE;
       element->which_element.source.has_break = FALSE;
       break;
@@ -537,7 +537,7 @@ init_win_info (struct tui_win_info * win
       win_info->detail.source_info.execution_info = (struct tui_gen_win_info *) NULL;
       win_info->detail.source_info.has_locator = FALSE;
       win_info->detail.source_info.horizontal_offset = 0;
-      win_info->detail.source_info.start_line_or_addr.addr = 0;
+      win_info->detail.source_info.start_line_or_addr = 0;
       win_info->detail.source_info.filename = 0;
       break;
     case DATA_WIN:
Index: src/gdb/tui/tui-disasm.c
===================================================================
--- src.orig/gdb/tui/tui-disasm.c	2005-10-20 18:16:44.000000000 +0100
+++ src/gdb/tui/tui-disasm.c	2005-10-21 15:31:23.000000000 +0100
@@ -188,7 +188,7 @@ tui_set_disassem_content (CORE_ADDR pc)
   if (ret != TUI_SUCCESS)
     return ret;
 
-  TUI_DISASM_WIN->detail.source_info.start_line_or_addr.addr = pc;
+  TUI_DISASM_WIN->detail.source_info.start_line_or_addr = pc;
   cur_pc = (CORE_ADDR)
     (((struct tui_win_element *) locator->content[0])->which_element.locator.addr);
 
@@ -249,7 +249,7 @@ tui_set_disassem_content (CORE_ADDR pc)
       else
         src->line[0] = '\0';
 
-      src->line_or_addr.addr = asm_lines[i].addr;
+      src->line_or_addr = asm_lines[i].addr;
       src->is_exec_point = asm_lines[i].addr == cur_pc;
 
       /* See whether there is a breakpoint installed.  */
@@ -270,9 +270,9 @@ tui_show_disassem (CORE_ADDR start_addr)
 {
   struct symtab *s = find_pc_symtab (start_addr);
   struct tui_win_info * win_with_focus = tui_win_with_focus ();
-  union tui_line_or_address val;
+  tui_line_or_address val;
 
-  val.addr = start_addr;
+  val = start_addr;
   tui_add_win_to_layout (DISASSEM_WIN);
   tui_update_source_window (TUI_DISASM_WIN, s, val, FALSE);
   /*
@@ -295,14 +295,14 @@ tui_show_disassem_and_update_source (COR
   tui_show_disassem (start_addr);
   if (tui_current_layout () == SRC_DISASSEM_COMMAND)
     {
-      union tui_line_or_address val;
+      tui_line_or_address val;
 
       /*
          ** Update what is in the source window if it is displayed too,
          ** note that it follows what is in the disassembly window and visa-versa
        */
       sal = find_pc_line (start_addr, 0);
-      val.line_no = sal.line;
+      val = sal.line;
       tui_update_source_window (TUI_SRC_WIN, sal.symtab, val, TRUE);
       if (sal.symtab)
 	{
@@ -376,7 +376,7 @@ tui_vertical_disassem_scroll (enum tui_s
       CORE_ADDR pc;
       tui_win_content content;
       struct symtab *s;
-      union tui_line_or_address val;
+      tui_line_or_address val;
       int max_lines, dir;
       struct symtab_and_line cursal = get_current_source_symtab_and_line ();
 
@@ -388,10 +388,10 @@ tui_vertical_disassem_scroll (enum tui_s
 
       /* account for hilite */
       max_lines = TUI_DISASM_WIN->generic.height - 2;
-      pc = content[0]->which_element.source.line_or_addr.addr;
+      pc = content[0]->which_element.source.line_or_addr;
       dir = (scroll_direction == FORWARD_SCROLL) ? max_lines : - max_lines;
 
-      val.addr = tui_find_disassembly_address (pc, dir);
+      val = tui_find_disassembly_address (pc, dir);
       tui_update_source_window_as_is (TUI_DISASM_WIN, s, val, FALSE);
     }
 }
Index: src/gdb/tui/tui-source.c
===================================================================
--- src.orig/gdb/tui/tui-source.c	2005-10-20 18:16:44.000000000 +0100
+++ src/gdb/tui/tui-source.c	2005-10-21 15:31:23.000000000 +0100
@@ -106,7 +106,7 @@ tui_set_source_content (struct symtab *s
 		  stream = fdopen (desc, FOPEN_RT);
 		  clearerr (stream);
 		  cur_line = 0;
-		  cur_line_no = src->start_line_or_addr.line_no = line_no;
+		  cur_line_no = src->start_line_or_addr = line_no;
 		  if (offset > 0)
 		    src_line = (char *) xmalloc (
 					   (threshold + 1) * sizeof (char));
@@ -137,8 +137,7 @@ tui_set_source_content (struct symtab *s
 
 		      /* Set whether element is the execution point and
 		         whether there is a break point on it.  */
-		      element->which_element.source.line_or_addr.line_no =
-			cur_line_no;
+		      element->which_element.source.line_or_addr = cur_line_no;
 		      element->which_element.source.is_exec_point =
 			(strcmp (((struct tui_win_element *)
 			locator->content[0])->which_element.locator.file_name,
@@ -247,7 +246,7 @@ tui_set_source_content_nil (struct tui_w
 
       struct tui_win_element * element =
       (struct tui_win_element *) win_info->generic.content[curr_line];
-      element->which_element.source.line_or_addr.line_no = 0;
+      element->which_element.source.line_or_addr = 0;
       element->which_element.source.is_exec_point = FALSE;
       element->which_element.source.has_break = FALSE;
 
@@ -295,7 +294,7 @@ tui_set_source_content_nil (struct tui_w
 /* Function to display source in the source window.  This function
    initializes the horizontal scroll to 0.  */
 void
-tui_show_symtab_source (struct symtab *s, union tui_line_or_address line, int noerror)
+tui_show_symtab_source (struct symtab *s, tui_line_or_address line, int noerror)
 {
   TUI_SRC_WIN->detail.source_info.horizontal_offset = 0;
   tui_update_source_window_as_is (TUI_SRC_WIN, s, line, noerror);
@@ -320,7 +319,7 @@ tui_vertical_source_scroll (enum tui_scr
 {
   if (TUI_SRC_WIN->generic.content != NULL)
     {
-      union tui_line_or_address l;
+      tui_line_or_address l;
       struct symtab *s;
       tui_win_content content = (tui_win_content) TUI_SRC_WIN->generic.content;
       struct symtab_and_line cursal = get_current_source_symtab_and_line ();
@@ -332,21 +331,19 @@ tui_vertical_source_scroll (enum tui_scr
 
       if (scroll_direction == FORWARD_SCROLL)
 	{
-	  l.line_no = content[0]->which_element.source.line_or_addr.line_no +
-	    num_to_scroll;
-	  if (l.line_no > s->nlines)
+	  l = content[0]->which_element.source.line_or_addr + num_to_scroll;
+	  if (l > s->nlines)
 	    /*line = s->nlines - win_info->generic.content_size + 1; */
 	    /*elz: fix for dts 23398 */
-	    l.line_no = content[0]->which_element.source.line_or_addr.line_no;
+	    l = content[0]->which_element.source.line_or_addr;
 	}
       else
 	{
-	  l.line_no = content[0]->which_element.source.line_or_addr.line_no -
-	    num_to_scroll;
-	  if (l.line_no <= 0)
-	    l.line_no = 1;
+	  l = content[0]->which_element.source.line_or_addr - num_to_scroll;
+	  if (l <= 0)
+	    l = 1;
 	}
 
-      print_source_lines (s, l.line_no, l.line_no + 1, 0);
+      print_source_lines (s, l, l + 1, 0);
     }
 }
Index: src/gdb/tui/tui-stack.c
===================================================================
--- src.orig/gdb/tui/tui-stack.c	2005-10-20 18:16:44.000000000 +0100
+++ src/gdb/tui/tui-stack.c	2005-10-21 15:31:23.000000000 +0100
@@ -364,14 +364,14 @@ tui_show_frame_info (struct frame_info *
 
 	  if (win_info == TUI_SRC_WIN)
 	    {
-	      union tui_line_or_address l;
-	      l.line_no = start_line;
+	      tui_line_or_address l;
+	      l = start_line;
 	      if (!(source_already_displayed
 		    && tui_line_is_displayed (item->locator.line_no, win_info, TRUE)))
 		tui_update_source_window (win_info, sal.symtab, l, TRUE);
 	      else
 		{
-		  l.line_no = item->locator.line_no;
+		  l = item->locator.line_no;
 		  tui_set_is_exec_point_at (l, win_info);
 		}
 	    }
@@ -379,13 +379,13 @@ tui_show_frame_info (struct frame_info *
 	    {
 	      if (win_info == TUI_DISASM_WIN)
 		{
-		  union tui_line_or_address a;
-		  a.addr = low;
+		  tui_line_or_address a;
+		  a = low;
 		  if (!tui_addr_is_displayed (item->locator.addr, win_info, TRUE))
 		    tui_update_source_window (win_info, sal.symtab, a, TRUE);
 		  else
 		    {
-		      a.addr = item->locator.addr;
+		      a = item->locator.addr;
 		      tui_set_is_exec_point_at (a, win_info);
 		    }
 		}
Index: src/gdb/tui/tui-win.c
===================================================================
--- src.orig/gdb/tui/tui-win.c	2005-10-20 18:16:44.000000000 +0100
+++ src/gdb/tui/tui-win.c	2005-10-21 15:32:09.000000000 +0100
@@ -1322,31 +1322,26 @@ make_visible_with_new_height (struct tui
       tui_make_visible (win_info->detail.source_info.execution_info);
       if (win_info->generic.content != NULL)
 	{
-	  union tui_line_or_address line_or_addr;
+	  tui_line_or_address line_or_addr;
 	  struct symtab_and_line cursal
 	    = get_current_source_symtab_and_line ();
 
-	  if (win_info->generic.type == SRC_WIN)
-	    line_or_addr.line_no =
-	      win_info->detail.source_info.start_line_or_addr.line_no;
-	  else
-	    line_or_addr.addr =
-	      win_info->detail.source_info.start_line_or_addr.addr;
+	  line_or_addr = win_info->detail.source_info.start_line_or_addr;
 	  tui_free_win_content (&win_info->generic);
 	  tui_update_source_window (win_info, cursal.symtab, line_or_addr, TRUE);
 	}
       else if (deprecated_selected_frame != (struct frame_info *) NULL)
 	{
-	  union tui_line_or_address line;
+	  tui_line_or_address line;
 	  struct symtab_and_line cursal = get_current_source_symtab_and_line ();
 
 
 	  s = find_pc_symtab (get_frame_pc (deprecated_selected_frame));
 	  if (win_info->generic.type == SRC_WIN)
-	    line.line_no = cursal.line;
+	    line = cursal.line;
 	  else
 	    {
-	      find_line_pc (s, cursal.line, &line.addr);
+	      find_line_pc (s, cursal.line, &line);
 	    }
 	  tui_update_source_window (win_info, s, line, TRUE);
 	}
Index: src/gdb/tui/tui-winsource.c
===================================================================
--- src.orig/gdb/tui/tui-winsource.c	2005-10-20 18:16:44.000000000 +0100
+++ src/gdb/tui/tui-winsource.c	2005-10-21 15:31:24.000000000 +0100
@@ -71,7 +71,7 @@ tui_display_main (void)
    initializes the horizontal scroll to 0.  */
 void
 tui_update_source_window (struct tui_win_info * win_info, struct symtab *s,
-			  union tui_line_or_address line_or_addr, int noerror)
+			  tui_line_or_address line_or_addr, int noerror)
 {
   win_info->detail.source_info.horizontal_offset = 0;
   tui_update_source_window_as_is (win_info, s, line_or_addr, noerror);
@@ -84,14 +84,14 @@ tui_update_source_window (struct tui_win
    shows the source as specified by the horizontal offset.  */
 void
 tui_update_source_window_as_is (struct tui_win_info * win_info, struct symtab *s,
-				union tui_line_or_address line_or_addr, int noerror)
+				tui_line_or_address line_or_addr, int noerror)
 {
   enum tui_status ret;
 
   if (win_info->generic.type == SRC_WIN)
-    ret = tui_set_source_content (s, line_or_addr.line_no, noerror);
+    ret = tui_set_source_content (s, line_or_addr, noerror);
   else
-    ret = tui_set_disassem_content (line_or_addr.addr);
+    ret = tui_set_disassem_content (line_or_addr);
 
   if (ret == TUI_FAILURE)
     {
@@ -107,8 +107,7 @@ tui_update_source_window_as_is (struct t
 	{
 	  struct symtab_and_line sal;
 	  
-	  sal.line = line_or_addr.line_no +
-	    (win_info->generic.content_size - 2);
+	  sal.line = line_or_addr + (win_info->generic.content_size - 2);
 	  sal.symtab = s;
 	  set_current_source_symtab_and_line (&sal);
 	  /*
@@ -134,7 +133,7 @@ tui_update_source_windows_with_addr (COR
   if (addr != 0)
     {
       struct symtab_and_line sal;
-      union tui_line_or_address l;
+      tui_line_or_address l;
       
       switch (tui_current_layout ())
 	{
@@ -147,7 +146,7 @@ tui_update_source_windows_with_addr (COR
 	  break;
 	default:
 	  sal = find_pc_line (addr, 0);
-	  l.line_no = sal.line;
+	  l = sal.line;
 	  tui_show_symtab_source (sal.symtab, l, FALSE);
 	  break;
 	}
@@ -172,7 +171,7 @@ void
 tui_update_source_windows_with_line (struct symtab *s, int line)
 {
   CORE_ADDR pc;
-  union tui_line_or_address l;
+  tui_line_or_address l;
   
   switch (tui_current_layout ())
     {
@@ -182,7 +181,7 @@ tui_update_source_windows_with_line (str
       tui_update_source_windows_with_addr (pc);
       break;
     default:
-      l.line_no = line;
+      l = line;
       tui_show_symtab_source (s, l, FALSE);
       if (tui_current_layout () == SRC_DISASSEM_COMMAND)
 	{
@@ -336,7 +335,7 @@ tui_horizontal_source_scroll (struct tui
 
 /* Set or clear the has_break flag in the line whose line is line_no.  */
 void
-tui_set_is_exec_point_at (union tui_line_or_address l, struct tui_win_info * win_info)
+tui_set_is_exec_point_at (tui_line_or_address l, struct tui_win_info * win_info)
 {
   int changed = 0;
   int i;
@@ -347,7 +346,7 @@ tui_set_is_exec_point_at (union tui_line
     {
       int new_state;
 
-      if (content[i]->which_element.source.line_or_addr.addr == l.addr)
+      if (content[i]->which_element.source.line_or_addr == l)
         new_state = TRUE;
       else
 	new_state = FALSE;
@@ -417,9 +416,9 @@ tui_update_breakpoint_info (struct tui_w
           if ((win == TUI_SRC_WIN
                && bp->source_file
                && (strcmp (src->filename, bp->source_file) == 0)
-               && bp->line_number == line->line_or_addr.line_no)
+               && bp->line_number == line->line_or_addr)
               || (win == TUI_DISASM_WIN
-                  && bp->loc->address == line->line_or_addr.addr))
+                  && bp->loc->address == line->line_or_addr))
             {
               if (bp->enable_state == bp_disabled)
                 mode |= TUI_BP_DISABLED;
@@ -614,7 +613,7 @@ tui_line_is_displayed (int line, struct 
   while (i < win_info->generic.content_size - threshold && !is_displayed)
     {
       is_displayed = (((struct tui_win_element *)
-		      win_info->generic.content[i])->which_element.source.line_or_addr.line_no
+		      win_info->generic.content[i])->which_element.source.line_or_addr
 		     == (int) line);
       i++;
     }
@@ -640,7 +639,7 @@ tui_addr_is_displayed (CORE_ADDR addr, s
   while (i < win_info->generic.content_size - threshold && !is_displayed)
     {
       is_displayed = (((struct tui_win_element *)
-		      win_info->generic.content[i])->which_element.source.line_or_addr.addr
+		      win_info->generic.content[i])->which_element.source.line_or_addr
 		     == addr);
       i++;
     }
Index: src/gdb/tui/tui-winsource.h
===================================================================
--- src.orig/gdb/tui/tui-winsource.h	2005-10-20 18:16:44.000000000 +0100
+++ src/gdb/tui/tui-winsource.h	2005-10-21 15:31:24.000000000 +0100
@@ -43,10 +43,10 @@ extern int tui_update_breakpoint_info (s
 /* Function to display the "main" routine.  */
 extern void tui_display_main (void);
 extern void tui_update_source_window (struct tui_win_info *, struct symtab *,
-				      union tui_line_or_address, int);
+				      tui_line_or_address, int);
 extern void tui_update_source_window_as_is (struct tui_win_info *,
 					    struct symtab *,
-					    union tui_line_or_address, int);
+					    tui_line_or_address, int);
 extern void tui_update_source_windows_with_addr (CORE_ADDR);
 extern void tui_update_source_windows_with_line (struct symtab *, int);
 extern void tui_clear_source_content (struct tui_win_info *, int);
@@ -60,7 +60,7 @@ extern void tui_erase_exec_info_content 
 extern void tui_clear_exec_info_content (struct tui_win_info *);
 extern void tui_update_exec_info (struct tui_win_info *);
 
-extern void tui_set_is_exec_point_at (union tui_line_or_address,
+extern void tui_set_is_exec_point_at (tui_line_or_address,
 				      struct tui_win_info *);
 extern enum tui_status tui_alloc_source_buffer (struct tui_win_info *);
 extern int tui_line_is_displayed (int, struct tui_win_info *, int);
Index: src/gdb/tui/tui-layout.c
===================================================================
--- src.orig/gdb/tui/tui-layout.c	2005-10-20 18:16:44.000000000 +0100
+++ src/gdb/tui/tui-layout.c	2005-10-21 15:31:24.000000000 +0100
@@ -519,14 +519,14 @@ extract_display_start_addr (void)
     case SRC_COMMAND:
     case SRC_DATA_COMMAND:
       find_line_pc (cursal.symtab,
-		    TUI_SRC_WIN->detail.source_info.start_line_or_addr.line_no,
+		    TUI_SRC_WIN->detail.source_info.start_line_or_addr,
 		    &pc);
       addr = pc;
       break;
     case DISASSEM_COMMAND:
     case SRC_DISASSEM_COMMAND:
     case DISASSEM_DATA_COMMAND:
-      addr = TUI_DISASM_WIN->detail.source_info.start_line_or_addr.addr;
+      addr = TUI_DISASM_WIN->detail.source_info.start_line_or_addr;
       break;
     default:
       addr = 0;
Index: src/gdb/tui/tui-source.h
===================================================================
--- src.orig/gdb/tui/tui-source.h	2005-10-20 18:16:44.000000000 +0100
+++ src/gdb/tui/tui-source.h	2005-10-21 15:31:24.000000000 +0100
@@ -33,7 +33,7 @@ struct tui_win_info;
 extern void tui_set_source_content_nil (struct tui_win_info *, char *);
 
 extern enum tui_status tui_set_source_content (struct symtab *, int, int);
-extern void tui_show_symtab_source (struct symtab *, union tui_line_or_address, int);
+extern void tui_show_symtab_source (struct symtab *, tui_line_or_address, int);
 extern int tui_source_is_displayed (char *);
 extern void tui_vertical_source_scroll (enum tui_scroll_direction, int);
 

[-- Attachment #3: tui-union-compare-2.patch --]
[-- Type: text/plain, Size: 20896 bytes --]

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

	* tui/tui-data.h (tui_line_or_address): Encapsulate the union in a
	struct with a tag.
	(tui_source_element, tui_source_info): Update.
	* tui/tui-disasm.c, tui/tui-source.c: Update to use the tagged union.
	* tui/tui-source.h, tui/tui-stack.c, tui/tui-win.c: Likewise.
	* tui/tui-winsource.c, tui/tui-data.c, tui/tui-layout.c: Likewise.
	* tui/tui-winsource.h: Likewise.


Index: src/gdb/tui/tui-data.h
===================================================================
--- src.orig/gdb/tui/tui-data.h	2005-10-20 15:08:54.000000000 +0100
+++ src/gdb/tui/tui-data.h	2005-10-20 17:37:21.000000000 +0100
@@ -147,10 +147,14 @@ enum tui_register_display_type
 };
 
 /* Structure describing source line or line address */
-union tui_line_or_address
+struct tui_line_or_address
 {
-  int line_no;
-  CORE_ADDR addr;
+  enum { LOA_LINE, LOA_ADDRESS } loa;
+  union
+    {
+      int line_no;
+      CORE_ADDR addr;
+    } u;
 };
 
 /* Current Layout definition */
@@ -166,7 +170,7 @@ struct tui_layout_def
 struct tui_source_element
 {
   char *line;
-  union tui_line_or_address line_or_addr;
+  struct tui_line_or_address line_or_addr;
   int is_exec_point;
   int has_break;
 };
@@ -259,7 +263,7 @@ struct tui_source_info
   /* Execution information window.  */
   struct tui_gen_win_info *execution_info;
   int horizontal_offset;	/* used for horizontal scroll */
-  union tui_line_or_address start_line_or_addr;
+  struct tui_line_or_address start_line_or_addr;
   char* filename;
 };
 
Index: src/gdb/tui/tui-disasm.c
===================================================================
--- src.orig/gdb/tui/tui-disasm.c	2005-10-20 15:08:54.000000000 +0100
+++ src/gdb/tui/tui-disasm.c	2005-10-20 17:43:25.000000000 +0100
@@ -188,7 +188,8 @@ tui_set_disassem_content (CORE_ADDR pc)
   if (ret != TUI_SUCCESS)
     return ret;
 
-  TUI_DISASM_WIN->detail.source_info.start_line_or_addr.addr = pc;
+  TUI_DISASM_WIN->detail.source_info.start_line_or_addr.loa = LOA_ADDRESS;
+  TUI_DISASM_WIN->detail.source_info.start_line_or_addr.u.addr = pc;
   cur_pc = (CORE_ADDR)
     (((struct tui_win_element *) locator->content[0])->which_element.locator.addr);
 
@@ -249,7 +250,8 @@ tui_set_disassem_content (CORE_ADDR pc)
       else
         src->line[0] = '\0';
 
-      src->line_or_addr.addr = asm_lines[i].addr;
+      src->line_or_addr.loa = LOA_ADDRESS;
+      src->line_or_addr.u.addr = asm_lines[i].addr;
       src->is_exec_point = asm_lines[i].addr == cur_pc;
 
       /* See whether there is a breakpoint installed.  */
@@ -270,9 +272,10 @@ tui_show_disassem (CORE_ADDR start_addr)
 {
   struct symtab *s = find_pc_symtab (start_addr);
   struct tui_win_info * win_with_focus = tui_win_with_focus ();
-  union tui_line_or_address val;
+  struct tui_line_or_address val;
 
-  val.addr = start_addr;
+  val.loa = LOA_ADDRESS;
+  val.u.addr = start_addr;
   tui_add_win_to_layout (DISASSEM_WIN);
   tui_update_source_window (TUI_DISASM_WIN, s, val, FALSE);
   /*
@@ -295,14 +298,15 @@ tui_show_disassem_and_update_source (COR
   tui_show_disassem (start_addr);
   if (tui_current_layout () == SRC_DISASSEM_COMMAND)
     {
-      union tui_line_or_address val;
+      struct tui_line_or_address val;
 
       /*
          ** Update what is in the source window if it is displayed too,
          ** note that it follows what is in the disassembly window and visa-versa
        */
       sal = find_pc_line (start_addr, 0);
-      val.line_no = sal.line;
+      val.loa = LOA_LINE;
+      val.u.line_no = sal.line;
       tui_update_source_window (TUI_SRC_WIN, sal.symtab, val, TRUE);
       if (sal.symtab)
 	{
@@ -376,7 +380,7 @@ tui_vertical_disassem_scroll (enum tui_s
       CORE_ADDR pc;
       tui_win_content content;
       struct symtab *s;
-      union tui_line_or_address val;
+      struct tui_line_or_address val;
       int max_lines, dir;
       struct symtab_and_line cursal = get_current_source_symtab_and_line ();
 
@@ -388,10 +392,11 @@ tui_vertical_disassem_scroll (enum tui_s
 
       /* account for hilite */
       max_lines = TUI_DISASM_WIN->generic.height - 2;
-      pc = content[0]->which_element.source.line_or_addr.addr;
+      pc = content[0]->which_element.source.line_or_addr.u.addr;
       dir = (scroll_direction == FORWARD_SCROLL) ? max_lines : - max_lines;
 
-      val.addr = tui_find_disassembly_address (pc, dir);
+      val.loa = LOA_ADDRESS;
+      val.u.addr = tui_find_disassembly_address (pc, dir);
       tui_update_source_window_as_is (TUI_DISASM_WIN, s, val, FALSE);
     }
 }
Index: src/gdb/tui/tui-source.c
===================================================================
--- src.orig/gdb/tui/tui-source.c	2005-10-20 15:08:54.000000000 +0100
+++ src/gdb/tui/tui-source.c	2005-10-20 17:51:04.000000000 +0100
@@ -106,7 +106,8 @@ tui_set_source_content (struct symtab *s
 		  stream = fdopen (desc, FOPEN_RT);
 		  clearerr (stream);
 		  cur_line = 0;
-		  cur_line_no = src->start_line_or_addr.line_no = line_no;
+		  src->start_line_or_addr.loa = LOA_LINE;
+		  cur_line_no = src->start_line_or_addr.u.line_no = line_no;
 		  if (offset > 0)
 		    src_line = (char *) xmalloc (
 					   (threshold + 1) * sizeof (char));
@@ -137,7 +138,9 @@ tui_set_source_content (struct symtab *s
 
 		      /* Set whether element is the execution point and
 		         whether there is a break point on it.  */
-		      element->which_element.source.line_or_addr.line_no =
+		      element->which_element.source.line_or_addr.loa =
+			LOA_LINE;
+		      element->which_element.source.line_or_addr.u.line_no =
 			cur_line_no;
 		      element->which_element.source.is_exec_point =
 			(strcmp (((struct tui_win_element *)
@@ -247,7 +250,8 @@ tui_set_source_content_nil (struct tui_w
 
       struct tui_win_element * element =
       (struct tui_win_element *) win_info->generic.content[curr_line];
-      element->which_element.source.line_or_addr.line_no = 0;
+      element->which_element.source.line_or_addr.loa = LOA_LINE;
+      element->which_element.source.line_or_addr.u.line_no = 0;
       element->which_element.source.is_exec_point = FALSE;
       element->which_element.source.has_break = FALSE;
 
@@ -295,7 +299,7 @@ tui_set_source_content_nil (struct tui_w
 /* Function to display source in the source window.  This function
    initializes the horizontal scroll to 0.  */
 void
-tui_show_symtab_source (struct symtab *s, union tui_line_or_address line, int noerror)
+tui_show_symtab_source (struct symtab *s, struct tui_line_or_address line, int noerror)
 {
   TUI_SRC_WIN->detail.source_info.horizontal_offset = 0;
   tui_update_source_window_as_is (TUI_SRC_WIN, s, line, noerror);
@@ -320,7 +324,7 @@ tui_vertical_source_scroll (enum tui_scr
 {
   if (TUI_SRC_WIN->generic.content != NULL)
     {
-      union tui_line_or_address l;
+      struct tui_line_or_address l;
       struct symtab *s;
       tui_win_content content = (tui_win_content) TUI_SRC_WIN->generic.content;
       struct symtab_and_line cursal = get_current_source_symtab_and_line ();
@@ -330,23 +334,24 @@ tui_vertical_source_scroll (enum tui_scr
       else
 	s = cursal.symtab;
 
+      l.loa = LOA_LINE;
       if (scroll_direction == FORWARD_SCROLL)
 	{
-	  l.line_no = content[0]->which_element.source.line_or_addr.line_no +
-	    num_to_scroll;
-	  if (l.line_no > s->nlines)
+	  l.u.line_no = content[0]->which_element.source.line_or_addr.u.line_no
+	    + num_to_scroll;
+	  if (l.u.line_no > s->nlines)
 	    /*line = s->nlines - win_info->generic.content_size + 1; */
 	    /*elz: fix for dts 23398 */
-	    l.line_no = content[0]->which_element.source.line_or_addr.line_no;
+	    l.u.line_no = content[0]->which_element.source.line_or_addr.u.line_no;
 	}
       else
 	{
-	  l.line_no = content[0]->which_element.source.line_or_addr.line_no -
-	    num_to_scroll;
-	  if (l.line_no <= 0)
-	    l.line_no = 1;
+	  l.u.line_no = content[0]->which_element.source.line_or_addr.u.line_no
+	    - num_to_scroll;
+	  if (l.u.line_no <= 0)
+	    l.u.line_no = 1;
 	}
 
-      print_source_lines (s, l.line_no, l.line_no + 1, 0);
+      print_source_lines (s, l.u.line_no, l.u.line_no + 1, 0);
     }
 }
Index: src/gdb/tui/tui-source.h
===================================================================
--- src.orig/gdb/tui/tui-source.h	2005-10-20 15:08:54.000000000 +0100
+++ src/gdb/tui/tui-source.h	2005-10-20 17:37:21.000000000 +0100
@@ -33,7 +33,7 @@ struct tui_win_info;
 extern void tui_set_source_content_nil (struct tui_win_info *, char *);
 
 extern enum tui_status tui_set_source_content (struct symtab *, int, int);
-extern void tui_show_symtab_source (struct symtab *, union tui_line_or_address, int);
+extern void tui_show_symtab_source (struct symtab *, struct tui_line_or_address, int);
 extern int tui_source_is_displayed (char *);
 extern void tui_vertical_source_scroll (enum tui_scroll_direction, int);
 
Index: src/gdb/tui/tui-stack.c
===================================================================
--- src.orig/gdb/tui/tui-stack.c	2005-10-20 15:08:54.000000000 +0100
+++ src/gdb/tui/tui-stack.c	2005-10-20 17:37:21.000000000 +0100
@@ -364,14 +364,15 @@ tui_show_frame_info (struct frame_info *
 
 	  if (win_info == TUI_SRC_WIN)
 	    {
-	      union tui_line_or_address l;
-	      l.line_no = start_line;
+	      struct tui_line_or_address l;
+	      l.loa = LOA_LINE;
+	      l.u.line_no = start_line;
 	      if (!(source_already_displayed
 		    && tui_line_is_displayed (item->locator.line_no, win_info, TRUE)))
 		tui_update_source_window (win_info, sal.symtab, l, TRUE);
 	      else
 		{
-		  l.line_no = item->locator.line_no;
+		  l.u.line_no = item->locator.line_no;
 		  tui_set_is_exec_point_at (l, win_info);
 		}
 	    }
@@ -379,13 +380,14 @@ tui_show_frame_info (struct frame_info *
 	    {
 	      if (win_info == TUI_DISASM_WIN)
 		{
-		  union tui_line_or_address a;
-		  a.addr = low;
+		  struct tui_line_or_address a;
+		  a.loa = LOA_ADDRESS;
+		  a.u.addr = low;
 		  if (!tui_addr_is_displayed (item->locator.addr, win_info, TRUE))
 		    tui_update_source_window (win_info, sal.symtab, a, TRUE);
 		  else
 		    {
-		      a.addr = item->locator.addr;
+		      a.u.addr = item->locator.addr;
 		      tui_set_is_exec_point_at (a, win_info);
 		    }
 		}
Index: src/gdb/tui/tui-win.c
===================================================================
--- src.orig/gdb/tui/tui-win.c	2005-10-20 15:08:54.000000000 +0100
+++ src/gdb/tui/tui-win.c	2005-10-20 17:37:21.000000000 +0100
@@ -1322,31 +1322,30 @@ make_visible_with_new_height (struct tui
       tui_make_visible (win_info->detail.source_info.execution_info);
       if (win_info->generic.content != NULL)
 	{
-	  union tui_line_or_address line_or_addr;
+	  struct tui_line_or_address line_or_addr;
 	  struct symtab_and_line cursal
 	    = get_current_source_symtab_and_line ();
 
-	  if (win_info->generic.type == SRC_WIN)
-	    line_or_addr.line_no =
-	      win_info->detail.source_info.start_line_or_addr.line_no;
-	  else
-	    line_or_addr.addr =
-	      win_info->detail.source_info.start_line_or_addr.addr;
+	  line_or_addr = win_info->detail.source_info.start_line_or_addr;
 	  tui_free_win_content (&win_info->generic);
 	  tui_update_source_window (win_info, cursal.symtab, line_or_addr, TRUE);
 	}
       else if (deprecated_selected_frame != (struct frame_info *) NULL)
 	{
-	  union tui_line_or_address line;
+	  struct tui_line_or_address line;
 	  struct symtab_and_line cursal = get_current_source_symtab_and_line ();
 
 
 	  s = find_pc_symtab (get_frame_pc (deprecated_selected_frame));
 	  if (win_info->generic.type == SRC_WIN)
-	    line.line_no = cursal.line;
+	    {
+	      line.loa = LOA_LINE;
+	      line.u.line_no = cursal.line;
+	    }
 	  else
 	    {
-	      find_line_pc (s, cursal.line, &line.addr);
+	      line.loa = LOA_ADDRESS;
+	      find_line_pc (s, cursal.line, &line.u.addr);
 	    }
 	  tui_update_source_window (win_info, s, line, TRUE);
 	}
Index: src/gdb/tui/tui-winsource.c
===================================================================
--- src.orig/gdb/tui/tui-winsource.c	2005-10-20 15:08:54.000000000 +0100
+++ src/gdb/tui/tui-winsource.c	2005-10-20 17:54:50.000000000 +0100
@@ -71,7 +71,7 @@ tui_display_main (void)
    initializes the horizontal scroll to 0.  */
 void
 tui_update_source_window (struct tui_win_info * win_info, struct symtab *s,
-			  union tui_line_or_address line_or_addr, int noerror)
+			  struct tui_line_or_address line_or_addr, int noerror)
 {
   win_info->detail.source_info.horizontal_offset = 0;
   tui_update_source_window_as_is (win_info, s, line_or_addr, noerror);
@@ -84,14 +84,14 @@ tui_update_source_window (struct tui_win
    shows the source as specified by the horizontal offset.  */
 void
 tui_update_source_window_as_is (struct tui_win_info * win_info, struct symtab *s,
-				union tui_line_or_address line_or_addr, int noerror)
+				struct tui_line_or_address line_or_addr, int noerror)
 {
   enum tui_status ret;
 
   if (win_info->generic.type == SRC_WIN)
-    ret = tui_set_source_content (s, line_or_addr.line_no, noerror);
+    ret = tui_set_source_content (s, line_or_addr.u.line_no, noerror);
   else
-    ret = tui_set_disassem_content (line_or_addr.addr);
+    ret = tui_set_disassem_content (line_or_addr.u.addr);
 
   if (ret == TUI_FAILURE)
     {
@@ -107,7 +107,7 @@ tui_update_source_window_as_is (struct t
 	{
 	  struct symtab_and_line sal;
 	  
-	  sal.line = line_or_addr.line_no +
+	  sal.line = line_or_addr.u.line_no +
 	    (win_info->generic.content_size - 2);
 	  sal.symtab = s;
 	  set_current_source_symtab_and_line (&sal);
@@ -134,7 +134,7 @@ tui_update_source_windows_with_addr (COR
   if (addr != 0)
     {
       struct symtab_and_line sal;
-      union tui_line_or_address l;
+      struct tui_line_or_address l;
       
       switch (tui_current_layout ())
 	{
@@ -147,7 +147,8 @@ tui_update_source_windows_with_addr (COR
 	  break;
 	default:
 	  sal = find_pc_line (addr, 0);
-	  l.line_no = sal.line;
+	  l.loa = LOA_LINE;
+	  l.u.line_no = sal.line;
 	  tui_show_symtab_source (sal.symtab, l, FALSE);
 	  break;
 	}
@@ -172,7 +173,7 @@ void
 tui_update_source_windows_with_line (struct symtab *s, int line)
 {
   CORE_ADDR pc;
-  union tui_line_or_address l;
+  struct tui_line_or_address l;
   
   switch (tui_current_layout ())
     {
@@ -182,7 +183,8 @@ tui_update_source_windows_with_line (str
       tui_update_source_windows_with_addr (pc);
       break;
     default:
-      l.line_no = line;
+      l.loa = LOA_LINE;
+      l.u.line_no = line;
       tui_show_symtab_source (s, l, FALSE);
       if (tui_current_layout () == SRC_DISASSEM_COMMAND)
 	{
@@ -336,7 +338,7 @@ tui_horizontal_source_scroll (struct tui
 
 /* Set or clear the has_break flag in the line whose line is line_no.  */
 void
-tui_set_is_exec_point_at (union tui_line_or_address l, struct tui_win_info * win_info)
+tui_set_is_exec_point_at (struct tui_line_or_address l, struct tui_win_info * win_info)
 {
   int changed = 0;
   int i;
@@ -347,7 +349,12 @@ tui_set_is_exec_point_at (union tui_line
     {
       int new_state;
 
-      if (content[i]->which_element.source.line_or_addr.addr == l.addr)
+      if (content[i]->which_element.source.line_or_addr.loa == l.loa
+	  && ((l.loa == LOA_LINE
+	       && content[i]->which_element.source.line_or_addr.u.line_no
+		  == l.u.line_no)
+              || (content[i]->which_element.source.line_or_addr.u.addr
+		  == l.u.addr)))
         new_state = TRUE;
       else
 	new_state = FALSE;
@@ -417,9 +424,9 @@ tui_update_breakpoint_info (struct tui_w
           if ((win == TUI_SRC_WIN
                && bp->source_file
                && (strcmp (src->filename, bp->source_file) == 0)
-               && bp->line_number == line->line_or_addr.line_no)
+               && bp->line_number == line->line_or_addr.u.line_no)
               || (win == TUI_DISASM_WIN
-                  && bp->loc->address == line->line_or_addr.addr))
+                  && bp->loc->address == line->line_or_addr.u.addr))
             {
               if (bp->enable_state == bp_disabled)
                 mode |= TUI_BP_DISABLED;
@@ -614,7 +621,7 @@ tui_line_is_displayed (int line, struct 
   while (i < win_info->generic.content_size - threshold && !is_displayed)
     {
       is_displayed = (((struct tui_win_element *)
-		      win_info->generic.content[i])->which_element.source.line_or_addr.line_no
+		      win_info->generic.content[i])->which_element.source.line_or_addr.u.line_no
 		     == (int) line);
       i++;
     }
@@ -640,7 +647,7 @@ tui_addr_is_displayed (CORE_ADDR addr, s
   while (i < win_info->generic.content_size - threshold && !is_displayed)
     {
       is_displayed = (((struct tui_win_element *)
-		      win_info->generic.content[i])->which_element.source.line_or_addr.addr
+		      win_info->generic.content[i])->which_element.source.line_or_addr.u.addr
 		     == addr);
       i++;
     }
Index: src/gdb/tui/tui-data.c
===================================================================
--- src.orig/gdb/tui/tui-data.c	2005-10-20 14:06:15.000000000 +0100
+++ src/gdb/tui/tui-data.c	2005-10-20 17:41:48.000000000 +0100
@@ -207,7 +207,8 @@ tui_clear_win_detail (struct tui_win_inf
 	{
 	case SRC_WIN:
 	case DISASSEM_WIN:
-	  win_info->detail.source_info.start_line_or_addr.addr = 0;
+	  win_info->detail.source_info.start_line_or_addr.loa = LOA_ADDRESS;
+	  win_info->detail.source_info.start_line_or_addr.u.addr = 0;
 	  win_info->detail.source_info.horizontal_offset = 0;
 	  break;
 	case CMD_WIN:
@@ -486,7 +487,8 @@ init_content_element (struct tui_win_ele
     case SRC_WIN:
     case DISASSEM_WIN:
       element->which_element.source.line = (char *) NULL;
-      element->which_element.source.line_or_addr.line_no = 0;
+      element->which_element.source.line_or_addr.loa = LOA_LINE;
+      element->which_element.source.line_or_addr.u.line_no = 0;
       element->which_element.source.is_exec_point = FALSE;
       element->which_element.source.has_break = FALSE;
       break;
@@ -537,7 +539,8 @@ init_win_info (struct tui_win_info * win
       win_info->detail.source_info.execution_info = (struct tui_gen_win_info *) NULL;
       win_info->detail.source_info.has_locator = FALSE;
       win_info->detail.source_info.horizontal_offset = 0;
-      win_info->detail.source_info.start_line_or_addr.addr = 0;
+      win_info->detail.source_info.start_line_or_addr.loa = LOA_ADDRESS;
+      win_info->detail.source_info.start_line_or_addr.u.addr = 0;
       win_info->detail.source_info.filename = 0;
       break;
     case DATA_WIN:
Index: src/gdb/tui/tui-layout.c
===================================================================
--- src.orig/gdb/tui/tui-layout.c	2005-10-20 14:06:15.000000000 +0100
+++ src/gdb/tui/tui-layout.c	2005-10-20 17:45:13.000000000 +0100
@@ -519,14 +519,14 @@ extract_display_start_addr (void)
     case SRC_COMMAND:
     case SRC_DATA_COMMAND:
       find_line_pc (cursal.symtab,
-		    TUI_SRC_WIN->detail.source_info.start_line_or_addr.line_no,
+		    TUI_SRC_WIN->detail.source_info.start_line_or_addr.u.line_no,
 		    &pc);
       addr = pc;
       break;
     case DISASSEM_COMMAND:
     case SRC_DISASSEM_COMMAND:
     case DISASSEM_DATA_COMMAND:
-      addr = TUI_DISASM_WIN->detail.source_info.start_line_or_addr.addr;
+      addr = TUI_DISASM_WIN->detail.source_info.start_line_or_addr.u.addr;
       break;
     default:
       addr = 0;
Index: src/gdb/tui/tui-winsource.h
===================================================================
--- src.orig/gdb/tui/tui-winsource.h	2005-10-20 14:06:15.000000000 +0100
+++ src/gdb/tui/tui-winsource.h	2005-10-20 17:39:00.000000000 +0100
@@ -43,10 +43,10 @@ extern int tui_update_breakpoint_info (s
 /* Function to display the "main" routine.  */
 extern void tui_display_main (void);
 extern void tui_update_source_window (struct tui_win_info *, struct symtab *,
-				      union tui_line_or_address, int);
+				      struct  tui_line_or_address, int);
 extern void tui_update_source_window_as_is (struct tui_win_info *,
 					    struct symtab *,
-					    union tui_line_or_address, int);
+					    struct  tui_line_or_address, int);
 extern void tui_update_source_windows_with_addr (CORE_ADDR);
 extern void tui_update_source_windows_with_line (struct symtab *, int);
 extern void tui_clear_source_content (struct tui_win_info *, int);
@@ -60,7 +60,7 @@ extern void tui_erase_exec_info_content 
 extern void tui_clear_exec_info_content (struct tui_win_info *);
 extern void tui_update_exec_info (struct tui_win_info *);
 
-extern void tui_set_is_exec_point_at (union tui_line_or_address,
+extern void tui_set_is_exec_point_at (struct  tui_line_or_address,
 				      struct tui_win_info *);
 extern enum tui_status tui_alloc_source_buffer (struct tui_win_info *);
 extern int tui_line_is_displayed (int, struct tui_win_info *, int);

Re: PATCH: Problem union comparision in TUI

[Daniel Jacobowitz]

On Fri, Oct 21, 2005 at 03:52:55PM +0100, Andrew STUBBS wrote:
> Eli Zaretskii wrote:
> >I still think that adding a tag to the union is the cleanest solution.
> 
> I have attached a patch implementing this. I have also attached a 
> slightly improved version of the one I posted before.
> 
> I am not entirely happy with the tagged union approach. There are a 
> number of places where it just assumes that the union must be doing the 
> right thing here. The only other possibility would be to assert. It 
> does, however, solve the problem at hand.
> 
> Does either of these grab your fancy?

Don't we always know what window the compared quantities belong to? 
Isn't which is in use a static property of the window type?  i.e. isn't
that a pre-existing "tag" for the union?

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: PATCH: Problem union comparision in TUI

[Eli Zaretskii]

> Date: Fri, 21 Oct 2005 12:39:11 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Eli Zaretskii <eliz@gnu.org>, gdb-patches@sources.redhat.com
> 
> Don't we always know what window the compared quantities belong to? 
> Isn't which is in use a static property of the window type?  i.e. isn't
> that a pre-existing "tag" for the union?

It didn't seem like that to me when I read the code, but perhaps I
missed something.

Re: PATCH: Problem union comparision in TUI

[Eli Zaretskii]

> Date: Fri, 21 Oct 2005 15:52:55 +0100
> From: Andrew STUBBS <andrew.stubbs@st.com>
> Cc: gdb-patches@sources.redhat.com
> 
> I have attached a patch implementing this. I have also attached a 
> slightly improved version of the one I posted before.

Thanks.

> I am not entirely happy with the tagged union approach. There are a 
> number of places where it just assumes that the union must be doing the 
> right thing here. The only other possibility would be to assert. It 
> does, however, solve the problem at hand.

I think we should gdb_assert usage of the right union member.

> Does either of these grab your fancy?

The second one (but see the comments below).

> @@ -347,7 +349,12 @@ tui_set_is_exec_point_at (union tui_line
>      {
>        int new_state;
>  
> -      if (content[i]->which_element.source.line_or_addr.addr == l.addr)
> +      if (content[i]->which_element.source.line_or_addr.loa == l.loa
> +	  && ((l.loa == LOA_LINE
> +	       && content[i]->which_element.source.line_or_addr.u.line_no
> +		  == l.u.line_no)
> +              || (content[i]->which_element.source.line_or_addr.u.addr
> +		  == l.u.addr)))
>          new_state = TRUE;
>        else

Why didn't you test l.loa to be LOA_ADDRESS before comparing the
addresses?

> @@ -417,9 +424,9 @@ tui_update_breakpoint_info (struct tui_w
>            if ((win == TUI_SRC_WIN
>                 && bp->source_file
>                 && (strcmp (src->filename, bp->source_file) == 0)
> -               && bp->line_number == line->line_or_addr.line_no)
> +               && bp->line_number == line->line_or_addr.u.line_no)
>                || (win == TUI_DISASM_WIN
> -                  && bp->loc->address == line->line_or_addr.addr))
> +                  && bp->loc->address == line->line_or_addr.u.addr))

Similarly here: I think the tag should be tested before you treat the
value as a lineno or an address.

> @@ -614,7 +621,7 @@ tui_line_is_displayed (int line, struct 
>    while (i < win_info->generic.content_size - threshold && !is_displayed)
>      {
>        is_displayed = (((struct tui_win_element *)
> -		      win_info->generic.content[i])->which_element.source.line_or_addr.line_no
> +		      win_info->generic.content[i])->which_element.source.line_or_addr.u.line_no
>  		     == (int) line);
>        i++;
>      }
> @@ -640,7 +647,7 @@ tui_addr_is_displayed (CORE_ADDR addr, s
>    while (i < win_info->generic.content_size - threshold && !is_displayed)
>      {
>        is_displayed = (((struct tui_win_element *)
> -		      win_info->generic.content[i])->which_element.source.line_or_addr.addr
> +		      win_info->generic.content[i])->which_element.source.line_or_addr.u.addr
>  		     == addr);
>        i++;
>      }

And here.

Re: PATCH: Problem union comparision in TUI

[Andrew STUBBS]

Eli Zaretskii wrote:
> Why didn't you test l.loa to be LOA_ADDRESS before comparing the
> addresses?

In theory it must be na address if it isn't a line. If it wasn't an 
address then what should the code do? An assert should fix this, same as 
the other issues.

I'll figure out how to use gdb_assert and post another patch later.

Thanks

Andrew

Re: PATCH: Problem union comparision in TUI

[Eli Zaretskii]

> Date: Mon, 24 Oct 2005 11:26:04 +0100
> From: Andrew STUBBS <andrew.stubbs@st.com>
> Cc: gdb-patches@sources.redhat.com
> 
> Eli Zaretskii wrote:
> > Why didn't you test l.loa to be LOA_ADDRESS before comparing the
> > addresses?
> 
> In theory it must be na address if it isn't a line. If it wasn't an 
> address then what should the code do?

It should use gdb_assert to signal a fatal internal error.  If that
error is ever reported, we will have a recipe to reproduce the bug,
and then we could fix it.

Re: PATCH: Problem union comparision in TUI

[Andrew STUBBS]

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

Eli Zaretskii wrote:
> It should use gdb_assert to signal a fatal internal error.  If that
> error is ever reported, we will have a recipe to reproduce the bug,
> and then we could fix it.

Updated patch attached.

[-- Attachment #2: tui-union-compare-2.patch --]
[-- Type: text/plain, Size: 21835 bytes --]

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

	* tui/tui-data.h (tui_line_or_address): Encapsulate the union in a
	struct with a tag.
	(tui_source_element, tui_source_info): Update.
	* tui/tui-disasm.c, tui/tui-source.c: Update to use the tagged union.
	* tui/tui-source.h, tui/tui-stack.c, tui/tui-win.c: Likewise.
	* tui/tui-winsource.c, tui/tui-data.c, tui/tui-layout.c: Likewise.
	* tui/tui-winsource.h: Likewise.


Index: src/gdb/tui/tui-data.h
===================================================================
--- src.orig/gdb/tui/tui-data.h	2005-10-24 12:53:31.000000000 +0100
+++ src/gdb/tui/tui-data.h	2005-10-24 12:55:05.000000000 +0100
@@ -147,10 +147,14 @@ enum tui_register_display_type
 };
 
 /* Structure describing source line or line address */
-union tui_line_or_address
+struct tui_line_or_address
 {
-  int line_no;
-  CORE_ADDR addr;
+  enum { LOA_LINE, LOA_ADDRESS } loa;
+  union
+    {
+      int line_no;
+      CORE_ADDR addr;
+    } u;
 };
 
 /* Current Layout definition */
@@ -166,7 +170,7 @@ struct tui_layout_def
 struct tui_source_element
 {
   char *line;
-  union tui_line_or_address line_or_addr;
+  struct tui_line_or_address line_or_addr;
   int is_exec_point;
   int has_break;
 };
@@ -259,7 +263,7 @@ struct tui_source_info
   /* Execution information window.  */
   struct tui_gen_win_info *execution_info;
   int horizontal_offset;	/* used for horizontal scroll */
-  union tui_line_or_address start_line_or_addr;
+  struct tui_line_or_address start_line_or_addr;
   char* filename;
 };
 
Index: src/gdb/tui/tui-disasm.c
===================================================================
--- src.orig/gdb/tui/tui-disasm.c	2005-10-24 12:53:32.000000000 +0100
+++ src/gdb/tui/tui-disasm.c	2005-10-24 12:55:05.000000000 +0100
@@ -188,7 +188,8 @@ tui_set_disassem_content (CORE_ADDR pc)
   if (ret != TUI_SUCCESS)
     return ret;
 
-  TUI_DISASM_WIN->detail.source_info.start_line_or_addr.addr = pc;
+  TUI_DISASM_WIN->detail.source_info.start_line_or_addr.loa = LOA_ADDRESS;
+  TUI_DISASM_WIN->detail.source_info.start_line_or_addr.u.addr = pc;
   cur_pc = (CORE_ADDR)
     (((struct tui_win_element *) locator->content[0])->which_element.locator.addr);
 
@@ -249,7 +250,8 @@ tui_set_disassem_content (CORE_ADDR pc)
       else
         src->line[0] = '\0';
 
-      src->line_or_addr.addr = asm_lines[i].addr;
+      src->line_or_addr.loa = LOA_ADDRESS;
+      src->line_or_addr.u.addr = asm_lines[i].addr;
       src->is_exec_point = asm_lines[i].addr == cur_pc;
 
       /* See whether there is a breakpoint installed.  */
@@ -270,9 +272,10 @@ tui_show_disassem (CORE_ADDR start_addr)
 {
   struct symtab *s = find_pc_symtab (start_addr);
   struct tui_win_info * win_with_focus = tui_win_with_focus ();
-  union tui_line_or_address val;
+  struct tui_line_or_address val;
 
-  val.addr = start_addr;
+  val.loa = LOA_ADDRESS;
+  val.u.addr = start_addr;
   tui_add_win_to_layout (DISASSEM_WIN);
   tui_update_source_window (TUI_DISASM_WIN, s, val, FALSE);
   /*
@@ -295,14 +298,15 @@ tui_show_disassem_and_update_source (COR
   tui_show_disassem (start_addr);
   if (tui_current_layout () == SRC_DISASSEM_COMMAND)
     {
-      union tui_line_or_address val;
+      struct tui_line_or_address val;
 
       /*
          ** Update what is in the source window if it is displayed too,
          ** note that it follows what is in the disassembly window and visa-versa
        */
       sal = find_pc_line (start_addr, 0);
-      val.line_no = sal.line;
+      val.loa = LOA_LINE;
+      val.u.line_no = sal.line;
       tui_update_source_window (TUI_SRC_WIN, sal.symtab, val, TRUE);
       if (sal.symtab)
 	{
@@ -376,7 +380,7 @@ tui_vertical_disassem_scroll (enum tui_s
       CORE_ADDR pc;
       tui_win_content content;
       struct symtab *s;
-      union tui_line_or_address val;
+      struct tui_line_or_address val;
       int max_lines, dir;
       struct symtab_and_line cursal = get_current_source_symtab_and_line ();
 
@@ -388,10 +392,11 @@ tui_vertical_disassem_scroll (enum tui_s
 
       /* account for hilite */
       max_lines = TUI_DISASM_WIN->generic.height - 2;
-      pc = content[0]->which_element.source.line_or_addr.addr;
+      pc = content[0]->which_element.source.line_or_addr.u.addr;
       dir = (scroll_direction == FORWARD_SCROLL) ? max_lines : - max_lines;
 
-      val.addr = tui_find_disassembly_address (pc, dir);
+      val.loa = LOA_ADDRESS;
+      val.u.addr = tui_find_disassembly_address (pc, dir);
       tui_update_source_window_as_is (TUI_DISASM_WIN, s, val, FALSE);
     }
 }
Index: src/gdb/tui/tui-source.c
===================================================================
--- src.orig/gdb/tui/tui-source.c	2005-10-24 12:53:32.000000000 +0100
+++ src/gdb/tui/tui-source.c	2005-10-24 12:55:05.000000000 +0100
@@ -106,7 +106,8 @@ tui_set_source_content (struct symtab *s
 		  stream = fdopen (desc, FOPEN_RT);
 		  clearerr (stream);
 		  cur_line = 0;
-		  cur_line_no = src->start_line_or_addr.line_no = line_no;
+		  src->start_line_or_addr.loa = LOA_LINE;
+		  cur_line_no = src->start_line_or_addr.u.line_no = line_no;
 		  if (offset > 0)
 		    src_line = (char *) xmalloc (
 					   (threshold + 1) * sizeof (char));
@@ -137,7 +138,9 @@ tui_set_source_content (struct symtab *s
 
 		      /* Set whether element is the execution point and
 		         whether there is a break point on it.  */
-		      element->which_element.source.line_or_addr.line_no =
+		      element->which_element.source.line_or_addr.loa =
+			LOA_LINE;
+		      element->which_element.source.line_or_addr.u.line_no =
 			cur_line_no;
 		      element->which_element.source.is_exec_point =
 			(strcmp (((struct tui_win_element *)
@@ -247,7 +250,8 @@ tui_set_source_content_nil (struct tui_w
 
       struct tui_win_element * element =
       (struct tui_win_element *) win_info->generic.content[curr_line];
-      element->which_element.source.line_or_addr.line_no = 0;
+      element->which_element.source.line_or_addr.loa = LOA_LINE;
+      element->which_element.source.line_or_addr.u.line_no = 0;
       element->which_element.source.is_exec_point = FALSE;
       element->which_element.source.has_break = FALSE;
 
@@ -295,7 +299,7 @@ tui_set_source_content_nil (struct tui_w
 /* Function to display source in the source window.  This function
    initializes the horizontal scroll to 0.  */
 void
-tui_show_symtab_source (struct symtab *s, union tui_line_or_address line, int noerror)
+tui_show_symtab_source (struct symtab *s, struct tui_line_or_address line, int noerror)
 {
   TUI_SRC_WIN->detail.source_info.horizontal_offset = 0;
   tui_update_source_window_as_is (TUI_SRC_WIN, s, line, noerror);
@@ -320,7 +324,7 @@ tui_vertical_source_scroll (enum tui_scr
 {
   if (TUI_SRC_WIN->generic.content != NULL)
     {
-      union tui_line_or_address l;
+      struct tui_line_or_address l;
       struct symtab *s;
       tui_win_content content = (tui_win_content) TUI_SRC_WIN->generic.content;
       struct symtab_and_line cursal = get_current_source_symtab_and_line ();
@@ -330,23 +334,24 @@ tui_vertical_source_scroll (enum tui_scr
       else
 	s = cursal.symtab;
 
+      l.loa = LOA_LINE;
       if (scroll_direction == FORWARD_SCROLL)
 	{
-	  l.line_no = content[0]->which_element.source.line_or_addr.line_no +
-	    num_to_scroll;
-	  if (l.line_no > s->nlines)
+	  l.u.line_no = content[0]->which_element.source.line_or_addr.u.line_no
+	    + num_to_scroll;
+	  if (l.u.line_no > s->nlines)
 	    /*line = s->nlines - win_info->generic.content_size + 1; */
 	    /*elz: fix for dts 23398 */
-	    l.line_no = content[0]->which_element.source.line_or_addr.line_no;
+	    l.u.line_no = content[0]->which_element.source.line_or_addr.u.line_no;
 	}
       else
 	{
-	  l.line_no = content[0]->which_element.source.line_or_addr.line_no -
-	    num_to_scroll;
-	  if (l.line_no <= 0)
-	    l.line_no = 1;
+	  l.u.line_no = content[0]->which_element.source.line_or_addr.u.line_no
+	    - num_to_scroll;
+	  if (l.u.line_no <= 0)
+	    l.u.line_no = 1;
 	}
 
-      print_source_lines (s, l.line_no, l.line_no + 1, 0);
+      print_source_lines (s, l.u.line_no, l.u.line_no + 1, 0);
     }
 }
Index: src/gdb/tui/tui-source.h
===================================================================
--- src.orig/gdb/tui/tui-source.h	2005-10-24 12:53:32.000000000 +0100
+++ src/gdb/tui/tui-source.h	2005-10-24 12:55:05.000000000 +0100
@@ -33,7 +33,7 @@ struct tui_win_info;
 extern void tui_set_source_content_nil (struct tui_win_info *, char *);
 
 extern enum tui_status tui_set_source_content (struct symtab *, int, int);
-extern void tui_show_symtab_source (struct symtab *, union tui_line_or_address, int);
+extern void tui_show_symtab_source (struct symtab *, struct tui_line_or_address, int);
 extern int tui_source_is_displayed (char *);
 extern void tui_vertical_source_scroll (enum tui_scroll_direction, int);
 
Index: src/gdb/tui/tui-stack.c
===================================================================
--- src.orig/gdb/tui/tui-stack.c	2005-10-24 12:53:32.000000000 +0100
+++ src/gdb/tui/tui-stack.c	2005-10-24 12:55:05.000000000 +0100
@@ -364,14 +364,15 @@ tui_show_frame_info (struct frame_info *
 
 	  if (win_info == TUI_SRC_WIN)
 	    {
-	      union tui_line_or_address l;
-	      l.line_no = start_line;
+	      struct tui_line_or_address l;
+	      l.loa = LOA_LINE;
+	      l.u.line_no = start_line;
 	      if (!(source_already_displayed
 		    && tui_line_is_displayed (item->locator.line_no, win_info, TRUE)))
 		tui_update_source_window (win_info, sal.symtab, l, TRUE);
 	      else
 		{
-		  l.line_no = item->locator.line_no;
+		  l.u.line_no = item->locator.line_no;
 		  tui_set_is_exec_point_at (l, win_info);
 		}
 	    }
@@ -379,13 +380,14 @@ tui_show_frame_info (struct frame_info *
 	    {
 	      if (win_info == TUI_DISASM_WIN)
 		{
-		  union tui_line_or_address a;
-		  a.addr = low;
+		  struct tui_line_or_address a;
+		  a.loa = LOA_ADDRESS;
+		  a.u.addr = low;
 		  if (!tui_addr_is_displayed (item->locator.addr, win_info, TRUE))
 		    tui_update_source_window (win_info, sal.symtab, a, TRUE);
 		  else
 		    {
-		      a.addr = item->locator.addr;
+		      a.u.addr = item->locator.addr;
 		      tui_set_is_exec_point_at (a, win_info);
 		    }
 		}
Index: src/gdb/tui/tui-win.c
===================================================================
--- src.orig/gdb/tui/tui-win.c	2005-10-24 12:53:32.000000000 +0100
+++ src/gdb/tui/tui-win.c	2005-10-24 12:55:05.000000000 +0100
@@ -1322,31 +1322,30 @@ make_visible_with_new_height (struct tui
       tui_make_visible (win_info->detail.source_info.execution_info);
       if (win_info->generic.content != NULL)
 	{
-	  union tui_line_or_address line_or_addr;
+	  struct tui_line_or_address line_or_addr;
 	  struct symtab_and_line cursal
 	    = get_current_source_symtab_and_line ();
 
-	  if (win_info->generic.type == SRC_WIN)
-	    line_or_addr.line_no =
-	      win_info->detail.source_info.start_line_or_addr.line_no;
-	  else
-	    line_or_addr.addr =
-	      win_info->detail.source_info.start_line_or_addr.addr;
+	  line_or_addr = win_info->detail.source_info.start_line_or_addr;
 	  tui_free_win_content (&win_info->generic);
 	  tui_update_source_window (win_info, cursal.symtab, line_or_addr, TRUE);
 	}
       else if (deprecated_selected_frame != (struct frame_info *) NULL)
 	{
-	  union tui_line_or_address line;
+	  struct tui_line_or_address line;
 	  struct symtab_and_line cursal = get_current_source_symtab_and_line ();
 
 
 	  s = find_pc_symtab (get_frame_pc (deprecated_selected_frame));
 	  if (win_info->generic.type == SRC_WIN)
-	    line.line_no = cursal.line;
+	    {
+	      line.loa = LOA_LINE;
+	      line.u.line_no = cursal.line;
+	    }
 	  else
 	    {
-	      find_line_pc (s, cursal.line, &line.addr);
+	      line.loa = LOA_ADDRESS;
+	      find_line_pc (s, cursal.line, &line.u.addr);
 	    }
 	  tui_update_source_window (win_info, s, line, TRUE);
 	}
Index: src/gdb/tui/tui-winsource.c
===================================================================
--- src.orig/gdb/tui/tui-winsource.c	2005-10-24 12:53:32.000000000 +0100
+++ src/gdb/tui/tui-winsource.c	2005-10-24 13:16:36.000000000 +0100
@@ -41,6 +41,7 @@
 
 #include "gdb_string.h"
 #include "gdb_curses.h"
+#include "gdb_assert.h"
 
 /* Function to display the "main" routine.  */
 void
@@ -71,7 +72,7 @@ tui_display_main (void)
    initializes the horizontal scroll to 0.  */
 void
 tui_update_source_window (struct tui_win_info * win_info, struct symtab *s,
-			  union tui_line_or_address line_or_addr, int noerror)
+			  struct tui_line_or_address line_or_addr, int noerror)
 {
   win_info->detail.source_info.horizontal_offset = 0;
   tui_update_source_window_as_is (win_info, s, line_or_addr, noerror);
@@ -84,14 +85,14 @@ tui_update_source_window (struct tui_win
    shows the source as specified by the horizontal offset.  */
 void
 tui_update_source_window_as_is (struct tui_win_info * win_info, struct symtab *s,
-				union tui_line_or_address line_or_addr, int noerror)
+				struct tui_line_or_address line_or_addr, int noerror)
 {
   enum tui_status ret;
 
   if (win_info->generic.type == SRC_WIN)
-    ret = tui_set_source_content (s, line_or_addr.line_no, noerror);
+    ret = tui_set_source_content (s, line_or_addr.u.line_no, noerror);
   else
-    ret = tui_set_disassem_content (line_or_addr.addr);
+    ret = tui_set_disassem_content (line_or_addr.u.addr);
 
   if (ret == TUI_FAILURE)
     {
@@ -107,7 +108,7 @@ tui_update_source_window_as_is (struct t
 	{
 	  struct symtab_and_line sal;
 	  
-	  sal.line = line_or_addr.line_no +
+	  sal.line = line_or_addr.u.line_no +
 	    (win_info->generic.content_size - 2);
 	  sal.symtab = s;
 	  set_current_source_symtab_and_line (&sal);
@@ -134,7 +135,7 @@ tui_update_source_windows_with_addr (COR
   if (addr != 0)
     {
       struct symtab_and_line sal;
-      union tui_line_or_address l;
+      struct tui_line_or_address l;
       
       switch (tui_current_layout ())
 	{
@@ -147,7 +148,8 @@ tui_update_source_windows_with_addr (COR
 	  break;
 	default:
 	  sal = find_pc_line (addr, 0);
-	  l.line_no = sal.line;
+	  l.loa = LOA_LINE;
+	  l.u.line_no = sal.line;
 	  tui_show_symtab_source (sal.symtab, l, FALSE);
 	  break;
 	}
@@ -172,7 +174,7 @@ void
 tui_update_source_windows_with_line (struct symtab *s, int line)
 {
   CORE_ADDR pc;
-  union tui_line_or_address l;
+  struct tui_line_or_address l;
   
   switch (tui_current_layout ())
     {
@@ -182,7 +184,8 @@ tui_update_source_windows_with_line (str
       tui_update_source_windows_with_addr (pc);
       break;
     default:
-      l.line_no = line;
+      l.loa = LOA_LINE;
+      l.u.line_no = line;
       tui_show_symtab_source (s, l, FALSE);
       if (tui_current_layout () == SRC_DISASSEM_COMMAND)
 	{
@@ -336,7 +339,7 @@ tui_horizontal_source_scroll (struct tui
 
 /* Set or clear the has_break flag in the line whose line is line_no.  */
 void
-tui_set_is_exec_point_at (union tui_line_or_address l, struct tui_win_info * win_info)
+tui_set_is_exec_point_at (struct tui_line_or_address l, struct tui_win_info * win_info)
 {
   int changed = 0;
   int i;
@@ -346,8 +349,15 @@ tui_set_is_exec_point_at (union tui_line
   while (i < win_info->generic.content_size)
     {
       int new_state;
+      struct tui_line_or_address content_loa =
+	content[i]->which_element.source.line_or_addr;
 
-      if (content[i]->which_element.source.line_or_addr.addr == l.addr)
+      gdb_assert (l.loa == LOA_ADDRESS || l.loa == LOA_LINE);
+      gdb_assert (content_loa.loa == LOA_LINE
+		  || content_loa.loa == LOA_ADDRESS);
+      if (content_loa.loa == l.loa
+	  && ((l.loa == LOA_LINE && content_loa.u.line_no == l.u.line_no)
+              || (content_loa.u.addr == l.u.addr)))
         new_state = TRUE;
       else
 	new_state = FALSE;
@@ -414,12 +424,16 @@ tui_update_breakpoint_info (struct tui_w
            bp != (struct breakpoint *) NULL;
            bp = bp->next)
         {
+	  gdb_assert (line->line_or_addr.loa == LOA_LINE
+		      || line->line_or_addr.loa == LOA_ADDRESS);
           if ((win == TUI_SRC_WIN
                && bp->source_file
                && (strcmp (src->filename, bp->source_file) == 0)
-               && bp->line_number == line->line_or_addr.line_no)
+	       && line->line_or_addr.loa == LOA_LINE
+               && bp->line_number == line->line_or_addr.u.line_no)
               || (win == TUI_DISASM_WIN
-                  && bp->loc->address == line->line_or_addr.addr))
+		  && line->line_or_addr.loa == LOA_ADDRESS
+                  && bp->loc->address == line->line_or_addr.u.addr))
             {
               if (bp->enable_state == bp_disabled)
                 mode |= TUI_BP_DISABLED;
@@ -614,8 +628,11 @@ tui_line_is_displayed (int line, struct 
   while (i < win_info->generic.content_size - threshold && !is_displayed)
     {
       is_displayed = (((struct tui_win_element *)
-		      win_info->generic.content[i])->which_element.source.line_or_addr.line_no
-		     == (int) line);
+		      win_info->generic.content[i])->which_element.source.line_or_addr.loa
+		      == LOA_LINE)
+		     && (((struct tui_win_element *)
+		      win_info->generic.content[i])->which_element.source.line_or_addr.u.line_no
+		      == (int) line);
       i++;
     }
 
@@ -640,8 +657,11 @@ tui_addr_is_displayed (CORE_ADDR addr, s
   while (i < win_info->generic.content_size - threshold && !is_displayed)
     {
       is_displayed = (((struct tui_win_element *)
-		      win_info->generic.content[i])->which_element.source.line_or_addr.addr
-		     == addr);
+		      win_info->generic.content[i])->which_element.source.line_or_addr.loa
+		      == LOA_ADDRESS)
+		     && (((struct tui_win_element *)
+		      win_info->generic.content[i])->which_element.source.line_or_addr.u.addr
+		      == addr);
       i++;
     }
 
Index: src/gdb/tui/tui-data.c
===================================================================
--- src.orig/gdb/tui/tui-data.c	2005-10-24 12:53:32.000000000 +0100
+++ src/gdb/tui/tui-data.c	2005-10-24 12:55:05.000000000 +0100
@@ -207,7 +207,8 @@ tui_clear_win_detail (struct tui_win_inf
 	{
 	case SRC_WIN:
 	case DISASSEM_WIN:
-	  win_info->detail.source_info.start_line_or_addr.addr = 0;
+	  win_info->detail.source_info.start_line_or_addr.loa = LOA_ADDRESS;
+	  win_info->detail.source_info.start_line_or_addr.u.addr = 0;
 	  win_info->detail.source_info.horizontal_offset = 0;
 	  break;
 	case CMD_WIN:
@@ -486,7 +487,8 @@ init_content_element (struct tui_win_ele
     case SRC_WIN:
     case DISASSEM_WIN:
       element->which_element.source.line = (char *) NULL;
-      element->which_element.source.line_or_addr.line_no = 0;
+      element->which_element.source.line_or_addr.loa = LOA_LINE;
+      element->which_element.source.line_or_addr.u.line_no = 0;
       element->which_element.source.is_exec_point = FALSE;
       element->which_element.source.has_break = FALSE;
       break;
@@ -537,7 +539,8 @@ init_win_info (struct tui_win_info * win
       win_info->detail.source_info.execution_info = (struct tui_gen_win_info *) NULL;
       win_info->detail.source_info.has_locator = FALSE;
       win_info->detail.source_info.horizontal_offset = 0;
-      win_info->detail.source_info.start_line_or_addr.addr = 0;
+      win_info->detail.source_info.start_line_or_addr.loa = LOA_ADDRESS;
+      win_info->detail.source_info.start_line_or_addr.u.addr = 0;
       win_info->detail.source_info.filename = 0;
       break;
     case DATA_WIN:
Index: src/gdb/tui/tui-layout.c
===================================================================
--- src.orig/gdb/tui/tui-layout.c	2005-10-24 12:53:32.000000000 +0100
+++ src/gdb/tui/tui-layout.c	2005-10-24 12:55:05.000000000 +0100
@@ -519,14 +519,14 @@ extract_display_start_addr (void)
     case SRC_COMMAND:
     case SRC_DATA_COMMAND:
       find_line_pc (cursal.symtab,
-		    TUI_SRC_WIN->detail.source_info.start_line_or_addr.line_no,
+		    TUI_SRC_WIN->detail.source_info.start_line_or_addr.u.line_no,
 		    &pc);
       addr = pc;
       break;
     case DISASSEM_COMMAND:
     case SRC_DISASSEM_COMMAND:
     case DISASSEM_DATA_COMMAND:
-      addr = TUI_DISASM_WIN->detail.source_info.start_line_or_addr.addr;
+      addr = TUI_DISASM_WIN->detail.source_info.start_line_or_addr.u.addr;
       break;
     default:
       addr = 0;
Index: src/gdb/tui/tui-winsource.h
===================================================================
--- src.orig/gdb/tui/tui-winsource.h	2005-10-24 12:53:32.000000000 +0100
+++ src/gdb/tui/tui-winsource.h	2005-10-24 12:55:05.000000000 +0100
@@ -43,10 +43,10 @@ extern int tui_update_breakpoint_info (s
 /* Function to display the "main" routine.  */
 extern void tui_display_main (void);
 extern void tui_update_source_window (struct tui_win_info *, struct symtab *,
-				      union tui_line_or_address, int);
+				      struct  tui_line_or_address, int);
 extern void tui_update_source_window_as_is (struct tui_win_info *,
 					    struct symtab *,
-					    union tui_line_or_address, int);
+					    struct  tui_line_or_address, int);
 extern void tui_update_source_windows_with_addr (CORE_ADDR);
 extern void tui_update_source_windows_with_line (struct symtab *, int);
 extern void tui_clear_source_content (struct tui_win_info *, int);
@@ -60,7 +60,7 @@ extern void tui_erase_exec_info_content 
 extern void tui_clear_exec_info_content (struct tui_win_info *);
 extern void tui_update_exec_info (struct tui_win_info *);
 
-extern void tui_set_is_exec_point_at (union tui_line_or_address,
+extern void tui_set_is_exec_point_at (struct  tui_line_or_address,
 				      struct tui_win_info *);
 extern enum tui_status tui_alloc_source_buffer (struct tui_win_info *);
 extern int tui_line_is_displayed (int, struct tui_win_info *, int);

Re: PATCH: Problem union comparision in TUI

[Eli Zaretskii]

> Date: Mon, 24 Oct 2005 13:53:44 +0100
> From: Andrew STUBBS <andrew.stubbs@st.com>
> Cc: gdb-patches@sources.redhat.com
> 
> Eli Zaretskii wrote:
> > It should use gdb_assert to signal a fatal internal error.  If that
> > error is ever reported, we will have a recipe to reproduce the bug,
> > and then we could fix it.
> 
> Updated patch attached.

Thanks, I'm happy now.

Does anyone object to this be committed?

Re: PATCH: Problem union comparision in TUI

[Andrew STUBBS]

Eli Zaretskii wrote:
> Thanks, I'm happy now.
> 
> Does anyone object to this be committed?

No objections yet. May I go ahead and commit it?

Andrew Stubbs

Re: PATCH: Problem union comparision in TUI

[Daniel Jacobowitz]

On Tue, Nov 01, 2005 at 04:22:26PM +0000, Andrew STUBBS wrote:
> Eli Zaretskii wrote:
> >Thanks, I'm happy now.
> >
> >Does anyone object to this be committed?
> 
> No objections yet. May I go ahead and commit it?

Yes, you may.

BTW, one gdb idiosyncrasy: please send commit messages to the list
after checking in patches.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: PATCH: Problem union comparision in TUI

[Andrew STUBBS]

Daniel Jacobowitz wrote:
> On Tue, Nov 01, 2005 at 04:22:26PM +0000, Andrew STUBBS wrote:
> 
>>Eli Zaretskii wrote:
>>
>>>Thanks, I'm happy now.
>>>
>>>Does anyone object to this be committed?
>>
>>No objections yet. May I go ahead and commit it?
> 
> 
> Yes, you may.
> 
> BTW, one gdb idiosyncrasy: please send commit messages to the list
> after checking in patches.
> 

Ok, I've commited this one now. ;)