RFA: Make cli-out follow gdb_stdout

RFA: Make cli-out follow gdb_stdout

[Daniel Jacobowitz]

Right now, when you create a cli_out object via cli_out_new, you pass the
value of gdb_stdout.  If we want to temporarily redirect output, that loses. 
Rather than temporarily changing the UI, I'd like to have cli_out_new follow
gdb_stdout.

There were two ways to do this:
  - hardcode the relationship between cli_out and gdb_stdout, since all
callers pass the same thing.
  - Pass &gdb_stdout instead of gdb_stdout.

I opted for the latter, as Pierre originally suggested.  If someone's got a
preference for the former I can switch easily enough.  I need this patch
before I can submit the code to support '>' and '>>', based on Tom Tromey's
patch from last October.

OK?

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

2002-07-16  Daniel Jacobowitz  <drow@mvista.com>

	* cli-out.c (struct ui_out_data): Change STREAM to
	`struct ui_file **'.
	(cli_field_fmt, cli_spaces, cli_text, cli_message)
	(cli_flush, out_field_fmt, field_separator): Dereference
	data->stream.
	(cli_out_new): Change argument to `struct ui_file **'.
	* cli-out.h (cli_out_new): Update prototype.
	* top.c (gdb_init): Update call to cli_out_new.

2002-07-16  Daniel Jacobowitz  <drow@mvista.com>

	* gdbtk/generic/gdbtk.c (gdbtk_init): Update call to cli_out_new.

2002-07-16  Daniel Jacobowitz  <drow@mvista.com>

	* tui/tuiIO.c (tui_initialize_io): Update call to cli_out_new.

Index: cli-out.c
===================================================================
RCS file: /cvs/src/src/gdb/cli-out.c,v
retrieving revision 1.14
diff -u -p -r1.14 cli-out.c
--- cli-out.c	19 Mar 2002 02:51:04 -0000	1.14
+++ cli-out.c	16 Jul 2002 16:25:11 -0000
@@ -30,7 +30,7 @@
 
 struct ui_out_data
   {
-    struct ui_file *stream;
+    struct ui_file **stream;
     int suppress_output;
   };
 
@@ -272,7 +272,7 @@ cli_field_fmt (struct ui_out *uiout, int
   if (data->suppress_output)
     return;
 
-  vfprintf_filtered (data->stream, format, args);
+  vfprintf_filtered (*data->stream, format, args);
 
   if (align != ui_noalign)
     field_separator ();
@@ -284,7 +284,7 @@ cli_spaces (struct ui_out *uiout, int nu
   struct ui_out_data *data = ui_out_data (uiout);
   if (data->suppress_output)
     return;
-  print_spaces_filtered (numspaces, data->stream);
+  print_spaces_filtered (numspaces, *data->stream);
 }
 
 void
@@ -293,7 +293,7 @@ cli_text (struct ui_out *uiout, const ch
   struct ui_out_data *data = ui_out_data (uiout);
   if (data->suppress_output)
     return;
-  fputs_filtered (string, data->stream);
+  fputs_filtered (string, *data->stream);
 }
 
 void
@@ -304,7 +304,7 @@ cli_message (struct ui_out *uiout, int v
   if (data->suppress_output)
     return;
   if (ui_out_get_verblvl (uiout) >= verbosity)
-    vfprintf_unfiltered (data->stream, format, args);
+    vfprintf_unfiltered (*data->stream, format, args);
 }
 
 void
@@ -320,7 +320,7 @@ void
 cli_flush (struct ui_out *uiout)
 {
   struct ui_out_data *data = ui_out_data (uiout);
-  gdb_flush (data->stream);
+  gdb_flush (*data->stream);
 }
 
 /* local functions */
@@ -338,7 +338,7 @@ out_field_fmt (struct ui_out *uiout, int
   va_list args;
 
   va_start (args, format);
-  vfprintf_filtered (data->stream, format, args);
+  vfprintf_filtered (*data->stream, format, args);
 
   va_end (args);
 }
@@ -349,13 +349,13 @@ static void
 field_separator (void)
 {
   struct ui_out_data *data = ui_out_data (uiout);
-  fputc_filtered (' ', data->stream);
+  fputc_filtered (' ', *data->stream);
 }
 
 /* initalize private members at startup */
 
 struct ui_out *
-cli_out_new (struct ui_file *stream)
+cli_out_new (struct ui_file **stream)
 {
   int flags = ui_source_list;
 
Index: cli-out.h
===================================================================
RCS file: /cvs/src/src/gdb/cli-out.h,v
retrieving revision 1.2
diff -u -p -r1.2 cli-out.h
--- cli-out.h	6 Mar 2001 08:21:06 -0000	1.2
+++ cli-out.h	16 Jul 2002 16:25:11 -0000
@@ -22,6 +22,6 @@
 #ifndef CLI_OUT_H
 #define CLI_OUT_H
 
-extern struct ui_out *cli_out_new (struct ui_file *stream);
+extern struct ui_out *cli_out_new (struct ui_file **stream);
 
 #endif
Index: top.c
===================================================================
RCS file: /cvs/src/src/gdb/top.c,v
retrieving revision 1.64
diff -u -p -r1.64 top.c
--- top.c	11 Jul 2002 13:50:49 -0000	1.64
+++ top.c	16 Jul 2002 16:25:12 -0000
@@ -2090,7 +2090,7 @@ gdb_init (char *argv0)
   /* Install the default UI */
   if (!init_ui_hook)
     {
-      uiout = cli_out_new (gdb_stdout);
+      uiout = cli_out_new (&gdb_stdout);
 
       /* All the interpreters should have had a look at things by now.
 	 Initialize the selected interpreter. */
Index: gdbtk/generic/gdbtk.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbtk/generic/gdbtk.c,v
retrieving revision 1.28
diff -u -p -r1.28 gdbtk.c
--- gdbtk/generic/gdbtk.c	17 Apr 2002 18:13:04 -0000	1.28
+++ gdbtk/generic/gdbtk.c	16 Jul 2002 16:25:13 -0000
@@ -587,7 +587,7 @@ gdbtk_init (char *argv0)
   gdb_stderr = gdbtk_fileopen ();
   gdb_stdlog = gdbtk_fileopen ();
   gdb_stdtarg = gdbtk_fileopen ();
-  uiout = cli_out_new (gdb_stdout);
+  uiout = cli_out_new (&gdb_stdout);
 
 #ifdef __CYGWIN32__
   (void) FreeConsole ();
Index: tui/tuiIO.c
===================================================================
RCS file: /cvs/src/src/gdb/tui/tuiIO.c,v
retrieving revision 1.12
diff -u -p -r1.12 tuiIO.c
--- tui/tuiIO.c	1 Mar 2002 06:19:28 -0000	1.12
+++ tui/tuiIO.c	16 Jul 2002 16:25:13 -0000
@@ -357,7 +357,7 @@ tui_initialize_io ()
 
   /* Create the default UI.  It is not created because we installed
      a init_ui_hook.  */
-  uiout = cli_out_new (gdb_stdout);
+  uiout = cli_out_new (&gdb_stdout);
 
   /* Temporary solution for readline writing to stdout:
      redirect readline output in a pipe, read that pipe and

Re: RFA: Make cli-out follow gdb_stdout

[Andrew Cagney]

FYI,

This:

>  struct ui_out_data
>    {
> -    struct ui_file *stream;
> +    struct ui_file **stream;
>      int suppress_output;
>    };

and this:

> -      uiout = cli_out_new (gdb_stdout);
> +      uiout = cli_out_new (&gdb_stdout);

worry me.

The intent is to eventually have everything using a parameterized output 
object - if you want to change where output goes, change the parameter. 
  Given that, having stuff depend on an indirect global pointer looks to 
be going in the wrong direction.

I'll need to look over the problems that Pierre was trying to solve.

Andrew

Re: RFA: Make cli-out follow gdb_stdout

[Daniel Jacobowitz]

On Mon, Jul 22, 2002 at 02:12:15PM -0400, Andrew Cagney wrote:
> FYI,
> 
> This:
> 
> > struct ui_out_data
> >   {
> >-    struct ui_file *stream;
> >+    struct ui_file **stream;
> >     int suppress_output;
> >   };
> 
> and this:
> 
> >-      uiout = cli_out_new (gdb_stdout);
> >+      uiout = cli_out_new (&gdb_stdout);
> 
> worry me.
> 
> The intent is to eventually have everything using a parameterized output 
> object - if you want to change where output goes, change the parameter. 
>  Given that, having stuff depend on an indirect global pointer looks to 
> be going in the wrong direction.
> 
> I'll need to look over the problems that Pierre was trying to solve.

I'm not thrilled with it myself.  Let me explain what I'm trying to do,
and let's see if we can come up with a better model.

I have a function which temporarily redirects GDB's output.  How does
it do this?  Well, the best way seems to be to modify
gdb_std{out,err,log}.  But the old value of gdb_stdout is cached in the
cli_out object.

The two minimal solutions were the one above (using a ui_file**) or
hardcoding gdb_stdout (since that's the only thing it's ever used for
at present).  They're both a bit of a step backwards.  I could provide
methods to query and set the underlying stream of a ui_out object, but
the differences between the different ui_out objects make that a little
awkward.  Would that be better?

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: RFA: Make cli-out follow gdb_stdout

[Andrew Cagney]

> I'm not thrilled with it myself.  Let me explain what I'm trying to do,
> and let's see if we can come up with a better model.
> 
> I have a function which temporarily redirects GDB's output.  How does
> it do this?  Well, the best way seems to be to modify
> gdb_std{out,err,log}.  But the old value of gdb_stdout is cached in the
> cli_out object.

So the assertion:

	global uiout->stream->ui_file == global gdb_stdout

doesn't hold :-(

> The two minimal solutions were the one above (using a ui_file**) or
> hardcoding gdb_stdout (since that's the only thing it's ever used for
> at present).  They're both a bit of a step backwards.  I could provide
> methods to query and set the underlying stream of a ui_out object, but
> the differences between the different ui_out objects make that a little
> awkward.  Would that be better?

Both of those are still wrong.  The global gdb_stdout should be going 
away replaced with some sort of explicitly parameterized i/o object. 
That is why catch_exceptions() takes a ui_out.

enjoy,
Andrew

Re: RFA: Make cli-out follow gdb_stdout

[Daniel Jacobowitz]

On Mon, Jul 22, 2002 at 03:18:53PM -0400, Andrew Cagney wrote:
> 
> >I'm not thrilled with it myself.  Let me explain what I'm trying to do,
> >and let's see if we can come up with a better model.
> >
> >I have a function which temporarily redirects GDB's output.  How does
> >it do this?  Well, the best way seems to be to modify
> >gdb_std{out,err,log}.  But the old value of gdb_stdout is cached in the
> >cli_out object.
> 
> So the assertion:
> 
> 	global uiout->stream->ui_file == global gdb_stdout
> 
> doesn't hold :-(
> 
> >The two minimal solutions were the one above (using a ui_file**) or
> >hardcoding gdb_stdout (since that's the only thing it's ever used for
> >at present).  They're both a bit of a step backwards.  I could provide
> >methods to query and set the underlying stream of a ui_out object, but
> >the differences between the different ui_out objects make that a little
> >awkward.  Would that be better?
> 
> Both of those are still wrong.  The global gdb_stdout should be going 
> away replaced with some sort of explicitly parameterized i/o object. 
> That is why catch_exceptions() takes a ui_out.

Well, that's not helpful in the short term :P

What do you envision by explicitly parametrized i/o object?  I really
don't think that's a good idea.  It would have to be passed down all
the way to every target function which might want to print some kind of
output.  That's a lot of bulk for no visible gain; somewhere down the
road if we supported multiple instantiations of the gdb library, maybe,
but we're so far away from that that this sort of direction seems
futile, until we are at least passing an object cookie everywhere.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: RFA: Make cli-out follow gdb_stdout

[Andrew Cagney]

[...]

> Well, that's not helpful in the short term :P
> 
> What do you envision by explicitly parametrized i/o object?  I really
> don't think that's a good idea.  It would have to be passed down all
> the way to every target function which might want to print some kind of
> output.  That's a lot of bulk for no visible gain; somewhere down the
> road if we supported multiple instantiations of the gdb library, maybe,
> but we're so far away from that that this sort of direction seems
> futile, until we are at least passing an object cookie everywhere.

As they say, I don't yet know.  I think it is pretty clear that the I/O 
is just one part of this state problem.  Since of thread, frame, 
register cache, ... are heavily dependant on state implemented with 
globals a guess is an object (or object relationship) that contains that 
state.   In the mean time we need to ensure that we're not entrenching 
the problem (eg by adding another bit of code assuming global state).

For the immediate problem, the intent is for catch_exceptions() to be 
used when overriding gdb's I/O.  Given, looking at the code, it too 
fails to keep that assertion (oops):

 > 	global uiout->stream->ui_file == global gdb_stdout

I think the thing to do is fix catch-exceptions and then use that.

enjoy,
Andrew

Re: RFA: Make cli-out follow gdb_stdout

[Daniel Jacobowitz]

On Mon, Jul 22, 2002 at 04:11:15PM -0400, Andrew Cagney wrote:
> 
> [...]
> 
> >Well, that's not helpful in the short term :P
> >
> >What do you envision by explicitly parametrized i/o object?  I really
> >don't think that's a good idea.  It would have to be passed down all
> >the way to every target function which might want to print some kind of
> >output.  That's a lot of bulk for no visible gain; somewhere down the
> >road if we supported multiple instantiations of the gdb library, maybe,
> >but we're so far away from that that this sort of direction seems
> >futile, until we are at least passing an object cookie everywhere.
> 
> As they say, I don't yet know.  I think it is pretty clear that the I/O 
> is just one part of this state problem.  Since of thread, frame, 
> register cache, ... are heavily dependant on state implemented with 
> globals a guess is an object (or object relationship) that contains that 
> state.   In the mean time we need to ensure that we're not entrenching 
> the problem (eg by adding another bit of code assuming global state).
> 
> For the immediate problem, the intent is for catch_exceptions() to be 
> used when overriding gdb's I/O.  Given, looking at the code, it too 
> fails to keep that assertion (oops):
> 
> > 	global uiout->stream->ui_file == global gdb_stdout
> 
> I think the thing to do is fix catch-exceptions and then use that.

catch_exceptions is for executing a particular command.  These commands
redirect the output of multiple commands; possibly an entire session.
catch_exceptions isn't built for that sort of use.

The only way that I could see to fix catch_exceptions in this regard
would involve some way to get gdb_stdout etc. from the uiout object.
In that case I'm just going to restrict my commands to CLI only, and
do:
  uiout = cli_out_new (new_gdb_stdout)
in the function.

Patch withdrawn until we figure out a better model here.  I bet you
aren't going to like my introduction of another wrapper much either...
but I fail to see an option.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer