Re: PATCH: Problem union comparision in TUI

Eli Zaretskii
2005-10-21T22:13:44Z

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