[PATCH] tui-out: don't "inherit" redirect from CLI

[PATCH] tui-out: don't "inherit" redirect from CLI

[Paul Bolle]

Since revision 1.17 tui_ui_out_impl "inherits" from cli_ui_out_impl (ie,
it starts as a copy of that struct). tui_ui_out_impl.redirect is never
redefined after that so it points to cli_redirect(). (Before revision
1.17 it was NULL.)

As a consequence, uo_redirect() will actually try to redirect if passed
a tui_ui_out_impl. That will probably crash gdb (as
tui_ui_out_impl->data->stream and tui_ui_out_impl->data->original_stream
don't exist).

So set tui_ui_out_impl.redirect to NULL after "inheriting".

(Just something I noticed while trying to understand the ui code.)
---
 gdb/ChangeLog     |    5 +++++
 gdb/tui/tui-out.c |    1 +
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 5d81848..1beb8fc 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2010-08-30  Paul Bolle  <pebolle@tiscali.nl>
+
+	* tui/tui-out.c (_initialize_tui_out): Don't "inherit" redirect from
+	CLI.
+
 2010-08-28  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	Code cleanup.
diff --git a/gdb/tui/tui-out.c b/gdb/tui/tui-out.c
index 8cd6b88..1e8ef74 100644
--- a/gdb/tui/tui-out.c
+++ b/gdb/tui/tui-out.c
@@ -178,4 +178,5 @@ _initialize_tui_out (void)
   tui_ui_out_impl.field_string = tui_field_string;
   tui_ui_out_impl.field_fmt = tui_field_fmt;
   tui_ui_out_impl.text = tui_text;
+  tui_ui_out_impl.redirect = NULL;
 }
-- 
1.7.2.1

Re: [PATCH] tui-out: don't "inherit" redirect from CLI

[Pedro Alves]

On Monday 30 August 2010 10:29:10, Paul Bolle wrote:
> Since revision 1.17 tui_ui_out_impl "inherits" from cli_ui_out_impl (ie,
> it starts as a copy of that struct). tui_ui_out_impl.redirect is never
> redefined after that so it points to cli_redirect(). (Before revision
> 1.17 it was NULL.)

That was the main reason that tui_ui_out_impl was made to inherit
cli_ui_out_impl in the first place.
See <http://sourceware.org/ml/gdb-patches/2010-04/msg00540.html>.

> As a consequence, uo_redirect() will actually try to redirect if passed
> a tui_ui_out_impl. That will probably crash gdb (as
> tui_ui_out_impl->data->stream and tui_ui_out_impl->data->original_stream
> don't exist).

Yes they do exist, because tui_ui_out_data "inherits" cli_ui_out_data.
See:

struct tui_ui_out_data
  {
    struct cli_ui_out_data base;
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^

    int line;
    int start_of_line;
  };
typedef struct tui_ui_out_data tui_out_data;

"base" is the first field of tui_ui_out_data, so you can
pass a pointer to a tui_ui_out_data to a function that expects
a cli_ui_out_data pointer.  This is legal, and a common idiom on C,
as means to implementing "is-a" inheritance.

> 
> So set tui_ui_out_impl.redirect to NULL after "inheriting".

Nope, sorry, this isn't correct.

> (Just something I noticed while trying to understand the ui code.)

-- 
Pedro Alves

Re: [PATCH] tui-out: don't "inherit" redirect from CLI

[Paul Bolle]

On Mon, 2010-08-30 at 12:29 +0100, Pedro Alves wrote:
> That was the main reason that tui_ui_out_impl was made to inherit
> cli_ui_out_impl in the first place.
> See <http://sourceware.org/ml/gdb-patches/2010-04/msg00540.html>.

Missed that one, obviously. (My excuse could be that the commit message
doesn't seem to mention that reason.)

> [...] tui_ui_out_data "inherits" cli_ui_out_data.
> See:
> 
> struct tui_ui_out_data
>   {
>     struct cli_ui_out_data base;
>     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
>     int line;
>     int start_of_line;
>   };
> typedef struct tui_ui_out_data tui_out_data;
> 
> "base" is the first field of tui_ui_out_data, so you can
> pass a pointer to a tui_ui_out_data to a function that expects
> a cli_ui_out_data pointer.  This is legal, and a common idiom on C,
> as means to implementing "is-a" inheritance.

Thanks for taking the time to explain this to me. Maybe next time I'll
recognize the idiom.


Paul