[patch] Fix nesting of ui_out_redirect

Jan Kratochvil
2010-09-03T11:21:52Z

Hi,

ui_out_redirect assumed only double level of redirections so far.

GDB started to use them nested, though. The testcase crashes FSF GDB HEAD.

Made there also some small related safety fixups of the related calls.

SUPPRESS_UI_FILEP_DECL is very ugly there but I am not aware how to easily use
vec.h otherwise and vec.h was being pushed as the GDB data type in such cases.
I would use just xmalloc()ed single LIFO link list otherwise.

No regressions on {x86_64,x86_64-m32,i686}-fedora14snapshot-linux-gnu.

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. Define SUPPRESS_UI_FILEP_DECL, ui_filep
and use DEF_VEC_P for ui_filep.
(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.
<! SUPPRESS_UI_FILEP_DECL>: Define ui_filep and use VEC_T.
(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. Move the comment into the inner block.
(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
@@ -22,6 +22,11 @@
along with this program. If not, see <http://www.gnu.org/licenses/>. */

#include "defs.h"
+#include "vec.h"
+#define SUPPRESS_UI_FILEP_DECL
+typedef struct ui_file *ui_filep;
+DEF_VEC_P (ui_filep);
+
#include "ui-out.h"
#include "cli-out.h"
#include "gdb_string.h"
@@ -224,11 +229,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 +245,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 +275,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 +298,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 +332,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 +347,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 +383,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 +407,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,20 @@
#define CLI_OUT_H

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

/* 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,9 +118,12 @@ 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)
- warning (_("Current output protocol does not support redirection"));
+ 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"));
+ }

if (logging_redirect != 0)
do_cleanups (cleanups);
@@ -212,7 +215,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:.*"