From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9015 invoked by alias); 22 Dec 2019 01:25:01 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 9007 invoked by uid 89); 22 Dec 2019 01:25:00 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-17.3 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=H*M:yahoo, H*x:Mozilla, H*x:Gecko, H*x:6.1 X-HELO: sonic311-32.consmr.mail.ir2.yahoo.com Received: from sonic311-32.consmr.mail.ir2.yahoo.com (HELO sonic311-32.consmr.mail.ir2.yahoo.com) (77.238.176.164) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 22 Dec 2019 01:24:58 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.de; s=s2048; t=1576977895; bh=Cugfg71lvG735JcK7xJWerJ9sis90R7jJFgun1cgPQ4=; h=Date:From:To:In-Reply-To:References:Subject:From:Subject; b=G9MYc+ixq6jRrcM9EPUpCscM6hdI4G0r9SXJqyjfMn8BGo7lru1iomttLn2TmZs2Y9ceigbC4rtHfD2SsBPnCyidr15tTj5QcYD9bc7MdqcqJO4pQAlA4ou6CIIaPfwMmmAxMVGqCauDg86V48TebMsxZaXzUuopmdngByfwYoTgyQ3U4K9D1duMW/RdtbOfWsRdrC7pdWMHXVRbvXQXdQpk8KAprQ84QptNstK15qz+NQJ0gFcB0QprYPMG3rUKa2nMpCvUGInlSZCZ0YrOkCKSfiYVxdzsNQJ0Xn4hy/6ScBmB4NlH/gEXJSk4hFRxj/bgUtN08pRQ7ntokqsmkQ== Received: from sonic.gate.mail.ne1.yahoo.com by sonic311.consmr.mail.ir2.yahoo.com with HTTP; Sun, 22 Dec 2019 01:24:55 +0000 Date: Sun, 22 Dec 2019 01:25:00 -0000 From: "Hannes Domani via gdb-patches" Reply-To: Hannes Domani To: Gdb-patches Message-ID: <771439547.3948471.1576977893095@mail.yahoo.com> In-Reply-To: <20191222005754.GF3865@embecosm.com> References: <20191221153325.6961-1-ssbssa.ref@yahoo.de> <20191221153325.6961-1-ssbssa@yahoo.de> <20191222005754.GF3865@embecosm.com> Subject: Re: [RFC][PATCH] Call tui_before_prompt in do_scroll_vertical MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2019-12/txt/msg00950.txt.bz2 Am Sonntag, 22. Dezember 2019, 01:57:56 MEZ hat Andrew Burgess Folgendes geschrieben: > * Hannes Domani via gdb-patches [2019-12-21 = 16:33:25 +0100]: > > > Without this call scrolling in the src window does not work at all. > > Thanks for spotting this and starting work on a fix. > > When fixing regressions like this its always useful to track down the > commit that introduced the failure and add a link to it.=C2=A0 This is not > about blaming others, but just adds a really useful historical log. > > In this case, it was this commit that caused the breakage in scrolling: > >=C2=A0=C2=A0 commit b4b49dcbff6b437fa8b4e2fc0c3f27b457f11310 (refs/bisect/= bad) >=C2=A0=C2=A0 Date:=C2=A0 Wed Nov 13 16:47:58 2019 -0700 > >=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Don't call tui_show_source from tui_u= i_out I have never tried bisecting before, and I don't know how it works on Windo= ws, but I'll try to do that the next time. > > > > First I tried it with tui_update_source_windows_with_line, but this did= n't > > reset from_source_symtab (which was set deep in print_source_lines), > > which resulted in some weird behavior when switching from "layout split" > > to "layout asm" after scrolling down in the src window (the asm window > > was then overwritten by the src window). > > I was able to reproduce the layout split -> layout asm issue you > describe, but, I'm not convinced that the solution below is the right > way to go. > > Tom might suggest a better solution quicker, but if not I'll try to > review this code properly soon. > > However, the questions/observations I have are: > > - It doesn't feel like calling a before prompt hook is the right way >=C2=A0=C2=A0 to go, and even if this is basically the right solution, we s= hould >=C2=A0=C2=A0 split the important part of the hook into a separate function= with >=C2=A0=C2=A0 a suitable name and call that instead. I wasn't 100% sure that this is the proper solution either, that's why I added [RFC]. > - I also don't understand why calling this hook in >=C2=A0=C2=A0 tui_souce_window::do_scroll_vertical changes what happens whe= n we >=C2=A0=C2=A0 switch to tui_disasm_window.=C2=A0 Stepping through from the = 'layout >=C2=A0=C2=A0 asm' seems to show that we do correctly change to the asm win= dow, >=C2=A0=C2=A0 and its only when we later call display_gdb_prompt (from >=C2=A0=C2=A0 event-top.c) that we overwrite the asm window with the source= for >=C2=A0=C2=A0 some reason.=C2=A0 I'd like to understand what's going on the= re more as >=C2=A0=C2=A0 it feels like if we could fix that as almost a separate issue= then >=C2=A0=C2=A0 we should go back to calling tui_update_source_windows_with_l= ine as >=C2=A0=C2=A0 you tried first, which feels like a better solution. That's what I tried to describe with this: > > First I tried it with tui_update_source_windows_with_line, but this did= n't > > reset from_source_symtab (which was set deep in print_source_lines), The variable from_source_symtab was set somewhere in print_source_lines, and because it was not reset, the next time tui_before_prompt is called, the source window is automatically added again in tui_refresh_frame_and_register_information. > - On coding style within GDB, we declare functions in header files, >=C2=A0=C2=A0 and define them in source files.=C2=A0 Also declarations are = marked >=C2=A0=C2=A0 extern and include return type and function name on one line,= so it >=C2=A0=C2=A0 would be: > >=C2=A0=C2=A0 extern void tui_before_prompt (const char *current_gdb_prompt= ); > >=C2=A0=C2=A0 But like I said, I'll need convincing that this is the right >=C2=A0=C2=A0 approach to fix this. Again, I agree with you that this is probably not the correct fix, that's why I added [RFC] and didn't bother with the function prototype in the correct header file. If that's not the proper use of [RFC], I apologize. Regards Hannes Domani > > Thanks, > Andrew > > > > > > gdb/ChangeLog: > > > > 2019-12-21=C2=A0 Hannes Domani=C2=A0 > > > >=C2=A0=C2=A0=C2=A0=C2=A0 * tui/tui-hooks.c (tui_before_prompt): Remove s= tatic. > >=C2=A0=C2=A0=C2=A0=C2=A0 * tui/tui-source.c (tui_before_prompt): Add pro= totype. > >=C2=A0=C2=A0=C2=A0=C2=A0 (tui_source_window::do_scroll_vertical): Add tu= i_before_prompt call. > > --- > >=C2=A0 gdb/tui/tui-hooks.c=C2=A0 | 2 +- > >=C2=A0 gdb/tui/tui-source.c | 5 +++++ > >=C2=A0 2 files changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c > > index 8576bb8fcc..8b9e70316f 100644 > > --- a/gdb/tui/tui-hooks.c > > +++ b/gdb/tui/tui-hooks.c > > @@ -179,7 +179,7 @@ tui_inferior_exit (struct inferior *inf) > > > >=C2=A0 /* Observer for the before_prompt notification.=C2=A0 */ > > > > -static void > > +void > >=C2=A0 tui_before_prompt (const char *current_gdb_prompt) > >=C2=A0 { > >=C2=A0=C2=A0=C2=A0 tui_refresh_frame_and_register_information (); > > diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c > > index 0728263b8c..dfe5721edf 100644 > > --- a/gdb/tui/tui-source.c > > +++ b/gdb/tui/tui-source.c > > @@ -39,6 +39,9 @@ > >=C2=A0 #include "tui/tui-source.h" > >=C2=A0 #include "gdb_curses.h" > > > > +void > > +tui_before_prompt (const char *current_gdb_prompt); > > + > >=C2=A0 /* Function to display source in the source window.=C2=A0 */ > >=C2=A0 bool > >=C2=A0 tui_source_window::set_contents (struct gdbarch *arch, > > @@ -158,6 +161,8 @@ tui_source_window::do_scroll_vertical (int num_to_s= croll) > >=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 l.u.line_no =3D 1; > > > >=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 print_source_lines (s, l.u.li= ne_no, l.u.line_no + 1, 0); > > + > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 tui_before_prompt (""); > >=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } > >=C2=A0 } > > > > -- > > 2.24.1 > > >