Re: PATCH: Problem union comparision in TUI

[Eli Zaretskii <eliz at gnu dot org>]

> 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.