From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22255 invoked by alias); 21 Oct 2005 22:13:44 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 22243 invoked by uid 22791); 21 Oct 2005 22:13:42 -0000 Received: from romy.inter.net.il (HELO romy.inter.net.il) (192.114.186.66) by sourceware.org (qpsmtpd/0.30-dev) with ESMTP; Fri, 21 Oct 2005 22:13:42 +0000 Received: from HOME-C4E4A596F7 (IGLD-80-230-90-20.inter.net.il [80.230.90.20]) by romy.inter.net.il (MOS 3.5.8-GR) with ESMTP id CTP39056 (AUTH halo1); Sat, 22 Oct 2005 00:13:38 +0200 (IST) Date: Fri, 21 Oct 2005 22:13:00 -0000 Message-Id: From: Eli Zaretskii To: Andrew STUBBS CC: gdb-patches@sources.redhat.com In-reply-to: <435900C7.2010706@st.com> (message from Andrew STUBBS on Fri, 21 Oct 2005 15:52:55 +0100) Subject: Re: PATCH: Problem union comparision in TUI Reply-to: Eli Zaretskii References: <4353BA69.1030401@st.com> <43561685.3010300@st.com> <20051019200751.GA19037@nevyn.them.org> <43576E68.8080804@st.com> <4357C346.8070400@st.com> <435900C7.2010706@st.com> X-SW-Source: 2005-10/txt/msg00180.txt.bz2 > Date: Fri, 21 Oct 2005 15:52:55 +0100 > From: Andrew STUBBS > 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.