Re: [patch] Fix nesting of ui_out_redirect

Jan Kratochvil
2010-09-03T15:23:54Z

On Fri, 03 Sep 2010 15:04:22 +0200, Pedro Alves wrote:
> Just putting the
>
> DEF_VEC_P (ui_filep);
>
> in the cli-out.h header is the norm.

I did not expect it is possible (I expected some link conflicts). I see it is
already being used in *.h files.

> It looks easy to tweak vec.h to get rid of the typedef, and so be able to
> forward declare VECs. E.g.,:

OK but that is not a part of this patch and the typedef is the GDB norm now.

> Each module that
> wants to use the VEC still needs to "DEF_VEC_P (ui_filep)"

This is not needed in this case, the structure is not opaque only to be
"inheritable" (embeddable) by other structs.

I tried to put there void * satisfying the sizeof but that does not work for
various reasons of its usage from vec.h.

> > + if (ui_out_redirect (uiout, NULL) < 0
> > + || ui_out_redirect (uiout, output) < 0)
> > + {
> > + /* It should not happen, the redirection has been already setup. */
> > + warning (_("Current output protocol does not support redirection"));
> > + }
>
> I'm feeling dense, and this change isn't looking obvious to me. Can you
> explain it?

Updated the comment, is it OK?

Thanks,
Jan

gdb/
2010-09-03 Jan Kratochvil <jan.kratochvil@redhat.com>

* breakpoint.c (save_breakpoints): Use RETURN_MASK_ALL.
* cli-out.c: Include vec.h.
(cli_field_fmt, cli_spaces, cli_text, cli_message, cli_flush): New
variable stream, initialize it, use it.
(cli_redirect): New function comment. Replace the stream and
original_stream fields by the new streams field. Remove the
original_stream != NULL conditional, assert error on NULL instead.
(out_field_fmt, field_separator): New variable stream, initialize it, use it.
(cli_out_data_ctor): Assert non-NULL stream. Replace the stream and
original_stream fields by the new streams field.
(cli_out_set_stream): Replace the stream field by the new streams
field.
* cli-out.h: Include vec.h.
(ui_filep): New typedef, call DEF_VEC_P for it.
(struct cli_ui_out_data): Replace the stream and original_stream
fields by the new streams field.
* cli/cli-logging.c (set_logging_redirect): Call ui_out_redirect with
NULL first. Extend the comment.
(handle_redirections): Call ui_out_redirect with output.
* python/py-breakpoint.c (bppy_get_commands): Move ui_out_redirect
calls outside of the TRY_CATCH block.

gdb/testsuite/
2010-09-03 Jan Kratochvil <jan.kratochvil@redhat.com>

* gdb.base/ui-redirect.exp: New file.

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -11487,7 +11487,7 @@ save_breakpoints (char *filename, int from_tty,
fprintf_unfiltered (fp, " commands\n");

ui_out_redirect (uiout, fp);
- TRY_CATCH (ex, RETURN_MASK_ERROR)
+ TRY_CATCH (ex, RETURN_MASK_ALL)
{
print_command_lines (uiout, tp->commands->commands, 2);
}
--- a/gdb/cli-out.c
+++ b/gdb/cli-out.c
@@ -26,6 +26,7 @@
#include "cli-out.h"
#include "gdb_string.h"
#include "gdb_assert.h"
+#include "vec.h"

typedef struct cli_ui_out_data cli_out_data;

@@ -224,11 +225,13 @@ cli_field_fmt (struct ui_out *uiout, int fldno,
va_list args)
{
cli_out_data *data = ui_out_data (uiout);
+ struct ui_file *stream;

if (data->suppress_output)
return;

- vfprintf_filtered (data->stream, format, args);
+ stream = VEC_last (ui_filep, data->streams);
+ vfprintf_filtered (stream, format, args);

if (align != ui_noalign)
field_separator ();
@@ -238,20 +241,26 @@ static void
cli_spaces (struct ui_out *uiout, int numspaces)
{
cli_out_data *data = ui_out_data (uiout);
+ struct ui_file *stream;

if (data->suppress_output)
return;
- print_spaces_filtered (numspaces, data->stream);
+
+ stream = VEC_last (ui_filep, data->streams);
+ print_spaces_filtered (numspaces, stream);
}

static void
cli_text (struct ui_out *uiout, const char *string)
{
cli_out_data *data = ui_out_data (uiout);
+ struct ui_file *stream;

if (data->suppress_output)
return;
- fputs_filtered (string, data->stream);
+
+ stream = VEC_last (ui_filep, data->streams);
+ fputs_filtered (string, stream);
}

static void ATTRIBUTE_PRINTF (3, 0)
@@ -262,8 +271,13 @@ cli_message (struct ui_out *uiout, int verbosity,

if (data->suppress_output)
return;
+
if (ui_out_get_verblvl (uiout) >= verbosity)
- vfprintf_unfiltered (data->stream, format, args);
+ {
+ struct ui_file *stream = VEC_last (ui_filep, data->streams);
+
+ vfprintf_unfiltered (stream, format, args);
+ }
}

static void
@@ -280,25 +294,24 @@ static void
cli_flush (struct ui_out *uiout)
{
cli_out_data *data = ui_out_data (uiout);
+ struct ui_file *stream = VEC_last (ui_filep, data->streams);

- gdb_flush (data->stream);
+ gdb_flush (stream);
}

+/* OUTSTREAM as non-NULL will push OUTSTREAM on the stack of output streams
+ and make it therefore active. OUTSTREAM as NULL will pop the last pushed
+ output stream; it is an internal error if it does not exist. */
+
static int
cli_redirect (struct ui_out *uiout, struct ui_file *outstream)
{
cli_out_data *data = ui_out_data (uiout);

if (outstream != NULL)
- {
- data->original_stream = data->stream;
- data->stream = outstream;
- }
- else if (data->original_stream != NULL)
- {
- data->stream = data->original_stream;
- data->original_stream = NULL;
- }
+ VEC_safe_push (ui_filep, data->streams, outstream);
+ else
+ VEC_pop (ui_filep, data->streams);

return 0;
}
@@ -315,10 +328,11 @@ out_field_fmt (struct ui_out *uiout, int fldno,
const char *format,...)
{
cli_out_data *data = ui_out_data (uiout);
+ struct ui_file *stream = VEC_last (ui_filep, data->streams);
va_list args;

va_start (args, format);
- vfprintf_filtered (data->stream, format, args);
+ vfprintf_filtered (stream, format, args);

va_end (args);
}
@@ -329,8 +343,9 @@ static void
field_separator (void)
{
cli_out_data *data = ui_out_data (uiout);
+ struct ui_file *stream = VEC_last (ui_filep, data->streams);

- fputc_filtered (' ', data->stream);
+ fputc_filtered (' ', stream);
}

/* This is the CLI ui-out implementation functions vector */
@@ -364,8 +379,11 @@ struct ui_out_impl cli_ui_out_impl =
void
cli_out_data_ctor (cli_out_data *self, struct ui_file *stream)
{
- self->stream = stream;
- self->original_stream = NULL;
+ gdb_assert (stream != NULL);
+
+ self->streams = NULL;
+ VEC_safe_push (ui_filep, self->streams, stream);
+
self->suppress_output = 0;
}

@@ -385,8 +403,10 @@ struct ui_file *
cli_out_set_stream (struct ui_out *uiout, struct ui_file *stream)
{
cli_out_data *data = ui_out_data (uiout);
- struct ui_file *old = data->stream;
+ struct ui_file *old;
+
+ old = VEC_pop (ui_filep, data->streams);
+ VEC_quick_push (ui_filep, data->streams, stream);

- data->stream = stream;
return old;
}
--- a/gdb/cli-out.h
+++ b/gdb/cli-out.h
@@ -22,14 +22,19 @@
#define CLI_OUT_H

#include "ui-out.h"
+#include "vec.h"
+
+/* Used for cli_ui_out_data->streams. */
+
+typedef struct ui_file *ui_filep;
+DEF_VEC_P (ui_filep);

/* These are exported so that they can be extended by other `ui_out'
implementations, like TUI's. */

struct cli_ui_out_data
{
- struct ui_file *stream;
- struct ui_file *original_stream;
+ VEC (ui_filep) *streams;
int suppress_output;
};

--- a/gdb/cli/cli-logging.c
+++ b/gdb/cli/cli-logging.c
@@ -118,8 +118,13 @@ set_logging_redirect (char *args, int from_tty, struct cmd_list_element *c)
gdb_stdtarg = output;
logging_no_redirect_file = new_logging_no_redirect_file;

- /* It should not happen, the redirection has been already setup. */
- if (ui_out_redirect (uiout, output) < 0)
+ /* There is a former output pushed on the ui_out_redirect stack. We want to
+ replace it by OUTPUT so we must pop the former value first. We should
+ either do both the pop and push or to do neither of it. At least do not
+ try to push OUTPUT if the pop already failed. */
+
+ if (ui_out_redirect (uiout, NULL) < 0
+ || ui_out_redirect (uiout, output) < 0)
warning (_("Current output protocol does not support redirection"));

if (logging_redirect != 0)
@@ -212,7 +217,7 @@ handle_redirections (int from_tty)
gdb_stdlog = output;
gdb_stdtarg = output;

- if (ui_out_redirect (uiout, gdb_stdout) < 0)
+ if (ui_out_redirect (uiout, output) < 0)
warning (_("Current output protocol does not support redirection"));
}

--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -474,12 +474,12 @@ bppy_get_commands (PyObject *self, void *closure)
string_file = mem_fileopen ();
chain = make_cleanup_ui_file_delete (string_file);

+ ui_out_redirect (uiout, string_file);
TRY_CATCH (except, RETURN_MASK_ALL)
{
- ui_out_redirect (uiout, string_file);
print_command_lines (uiout, breakpoint_commands (bp), 0);
- ui_out_redirect (uiout, NULL);
}
+ ui_out_redirect (uiout, NULL);
cmdstr = ui_file_xstrdup (string_file, &length);
GDB_PY_HANDLE_EXCEPTION (except);

--- /dev/null
+++ b/gdb/testsuite/gdb.base/ui-redirect.exp
@@ -0,0 +1,41 @@
+# Copyright (C) 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+if { [prepare_for_testing ui-redirect.exp ui-redirect start.c] } {
+ return -1
+}
+
+gdb_breakpoint main
+
+set test "commands"
+gdb_test_multiple $test $test {
+ -re "End with a line saying just \"end\"\\.\r\n>$" {
+ pass $test
+ }
+}
+
+set test "print 1"
+gdb_test_multiple $test $test {
+ -re "\r\n>$" {
+ pass $test
+ }
+}
+gdb_test_no_output "end"
+
+gdb_test_no_output "set logging file /dev/null"
+gdb_test "set logging on" "Copying output to /dev/null\\."
+gdb_test "save breakpoints /dev/null" "Saved to file '/dev/null'\\."
+gdb_test "set logging off" "Done logging to /dev/null\\."
+gdb_test "help" "List of classes of commands:.*"