From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1365 invoked by alias); 26 Sep 2014 09:25:26 -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 1352 invoked by uid 89); 26 Sep 2014 09:25:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_00,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-la0-f41.google.com Received: from mail-la0-f41.google.com (HELO mail-la0-f41.google.com) (209.85.215.41) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Fri, 26 Sep 2014 09:25:23 +0000 Received: by mail-la0-f41.google.com with SMTP id s18so14285236lam.28 for ; Fri, 26 Sep 2014 02:25:19 -0700 (PDT) X-Received: by 10.152.10.2 with SMTP id e2mr1552318lab.96.1411723519131; Fri, 26 Sep 2014 02:25:19 -0700 (PDT) Received: from smtp.gmail.com ([141.85.227.189]) by mx.google.com with ESMTPSA id ki7sm1679019lac.38.2014.09.26.02.25.17 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 26 Sep 2014 02:25:18 -0700 (PDT) From: Adrian Sendroiu To: Pedro Alves Cc: tromey@redhat.com, gdb-patches@sourceware.org Subject: Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack. References: <87iokwc2sl.fsf@gmail.com> Date: Fri, 26 Sep 2014 09:25:00 -0000 In-Reply-To: <87iokwc2sl.fsf@gmail.com> (Adrian Sendroiu's message of "Tue, 09 Sep 2014 16:59:54 +0300") Message-ID: <87ppeivina.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2014-09/txt/msg00766.txt.bz2 ping Adrian Sendroiu writes: >> I think it'd be good to replace this ".*" with a stricter match, just >> in case something goes wrong with undoing the redirection, but nothing >> crashes. Something like (untested): >> >> mi_gdb_test "help" "List of classes of commands.*\\^done.*" "nested redirect" >> >> This is OK with a change along those lines. > > Ok, I changed to mi_gdb_test "help" ".*List of classes of commands.*\\^done" "nested redirect" > > Adrian > > --- 8< --- > Subject: [PATCH] mi-out: Implement mi redirection using a stack. > > The current implementation doesn't support nested redirections because it only > has an "original_buffer" where it saves the current stream when a redirection is > done. To overcome this limitation, this patch reimplements it using a stack of > streams. > > gdb/ > 2014-07-23 Adrian Sendroiu > > * mi/mi-out.c (ui_filep): New typedef. > (DEF_VEC_P (ui_filep)): New type. > (struct ui_out_data): Remove field. > Remove field. > : New field. > (mi_field_string): Get the output stream from the top of the > stack of streams. > (mi_field_fmt): Likewise. > (mi_flush): Likewise. > (field_separator): Likewise. > (mi_open): Likewise. > (mi_close): Likewise. > (mi_out_buffered): Likewise. > (mi_out_rewind): Likewise. > (mi_out_put): Likewise. > (mi_out_new): Initialize the stack of streams. Push a newly > created stream on the stack. Remove obsolete comment. > (mi_redirect): Reimplement using push and pop operations. > > gdb/testsuite/ > 2014-08-05 Adrian Sendroiu > > * gdb.mi/mi-logging.exp: Add test for nested mi redirection. > * lib/gdb.exp: (skip_python_tests_prompt): New function. > (skip_python_tests): Use skip_python_tests_prompt. > * lib/mi-support.exp: (mi_skip_python_tests): New function. > --- > gdb/mi/mi-out.c | 77 +++++++++++++++++++++---------------- > gdb/testsuite/gdb.mi/mi-logging.exp | 14 +++++++ > gdb/testsuite/lib/gdb.exp | 20 ++++++---- > gdb/testsuite/lib/mi-support.exp | 7 ++++ > 4 files changed, 77 insertions(+), 41 deletions(-) > > diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c > index 6ec41e6..6731509 100644 > --- a/gdb/mi/mi-out.c > +++ b/gdb/mi/mi-out.c > @@ -22,14 +22,17 @@ > #include "defs.h" > #include "ui-out.h" > #include "mi-out.h" > +#include "vec.h" > + > +typedef struct ui_file *ui_filep; > +DEF_VEC_P (ui_filep); > > struct ui_out_data > { > int suppress_field_separator; > int suppress_output; > int mi_version; > - struct ui_file *buffer; > - struct ui_file *original_buffer; > + VEC (ui_filep) *streams; > }; > typedef struct ui_out_data mi_out_data; > > @@ -215,17 +218,20 @@ mi_field_string (struct ui_out *uiout, int fldno, int width, > enum ui_align align, const char *fldname, const char *string) > { > mi_out_data *data = ui_out_data (uiout); > + struct ui_file *stream; > > if (data->suppress_output) > return; > > + stream = VEC_last (ui_filep, data->streams); > + > field_separator (uiout); > if (fldname) > - fprintf_unfiltered (data->buffer, "%s=", fldname); > - fprintf_unfiltered (data->buffer, "\""); > + fprintf_unfiltered (stream, "%s=", fldname); > + fprintf_unfiltered (stream, "\""); > if (string) > - fputstr_unfiltered (string, '"', data->buffer); > - fprintf_unfiltered (data->buffer, "\""); > + fputstr_unfiltered (string, '"', stream); > + fprintf_unfiltered (stream, "\""); > } > > /* This is the only field function that does not align. */ > @@ -236,17 +242,20 @@ mi_field_fmt (struct ui_out *uiout, int fldno, int width, > const char *format, va_list args) > { > mi_out_data *data = ui_out_data (uiout); > + struct ui_file *stream; > > if (data->suppress_output) > return; > > + stream = VEC_last (ui_filep, data->streams); > + > field_separator (uiout); > if (fldname) > - fprintf_unfiltered (data->buffer, "%s=\"", fldname); > + fprintf_unfiltered (stream, "%s=\"", fldname); > else > - fputs_unfiltered ("\"", data->buffer); > - vfprintf_unfiltered (data->buffer, format, args); > - fputs_unfiltered ("\"", data->buffer); > + fputs_unfiltered ("\"", stream); > + vfprintf_unfiltered (stream, format, args); > + fputs_unfiltered ("\"", stream); > } > > void > @@ -275,8 +284,9 @@ void > mi_flush (struct ui_out *uiout) > { > mi_out_data *data = ui_out_data (uiout); > + struct ui_file *stream = VEC_last (ui_filep, data->streams); > > - gdb_flush (data->buffer); > + gdb_flush (stream); > } > > int > @@ -285,15 +295,9 @@ mi_redirect (struct ui_out *uiout, struct ui_file *outstream) > mi_out_data *data = ui_out_data (uiout); > > if (outstream != NULL) > - { > - data->original_buffer = data->buffer; > - data->buffer = outstream; > - } > - else if (data->original_buffer != NULL) > - { > - data->buffer = data->original_buffer; > - data->original_buffer = NULL; > - } > + VEC_safe_push (ui_filep, data->streams, outstream); > + else > + VEC_pop (ui_filep, data->streams); > > return 0; > } > @@ -306,29 +310,31 @@ static void > field_separator (struct ui_out *uiout) > { > mi_out_data *data = ui_out_data (uiout); > + struct ui_file *stream = VEC_last (ui_filep, data->streams); > > if (data->suppress_field_separator) > data->suppress_field_separator = 0; > else > - fputc_unfiltered (',', data->buffer); > + fputc_unfiltered (',', stream); > } > > static void > mi_open (struct ui_out *uiout, const char *name, enum ui_out_type type) > { > mi_out_data *data = ui_out_data (uiout); > + struct ui_file *stream = VEC_last (ui_filep, data->streams); > > field_separator (uiout); > data->suppress_field_separator = 1; > if (name) > - fprintf_unfiltered (data->buffer, "%s=", name); > + fprintf_unfiltered (stream, "%s=", name); > switch (type) > { > case ui_out_type_tuple: > - fputc_unfiltered ('{', data->buffer); > + fputc_unfiltered ('{', stream); > break; > case ui_out_type_list: > - fputc_unfiltered ('[', data->buffer); > + fputc_unfiltered ('[', stream); > break; > default: > internal_error (__FILE__, __LINE__, _("bad switch")); > @@ -339,14 +345,15 @@ static void > mi_close (struct ui_out *uiout, enum ui_out_type type) > { > mi_out_data *data = ui_out_data (uiout); > + struct ui_file *stream = VEC_last (ui_filep, data->streams); > > switch (type) > { > case ui_out_type_tuple: > - fputc_unfiltered ('}', data->buffer); > + fputc_unfiltered ('}', stream); > break; > case ui_out_type_list: > - fputc_unfiltered (']', data->buffer); > + fputc_unfiltered (']', stream); > break; > default: > internal_error (__FILE__, __LINE__, _("bad switch")); > @@ -360,8 +367,9 @@ void > mi_out_buffered (struct ui_out *uiout, char *string) > { > mi_out_data *data = ui_out_data (uiout); > + struct ui_file *stream = VEC_last (ui_filep, data->streams); > > - fprintf_unfiltered (data->buffer, "%s", string); > + fprintf_unfiltered (stream, "%s", string); > } > > /* Clear the buffer. */ > @@ -370,8 +378,9 @@ void > mi_out_rewind (struct ui_out *uiout) > { > mi_out_data *data = ui_out_data (uiout); > + struct ui_file *stream = VEC_last (ui_filep, data->streams); > > - ui_file_rewind (data->buffer); > + ui_file_rewind (stream); > } > > /* Dump the buffer onto the specified stream. */ > @@ -386,9 +395,10 @@ void > mi_out_put (struct ui_out *uiout, struct ui_file *stream) > { > mi_out_data *data = ui_out_data (uiout); > + struct ui_file *current_stream = VEC_last (ui_filep, data->streams); > > - ui_file_put (data->buffer, do_write, stream); > - ui_file_rewind (data->buffer); > + ui_file_put (current_stream, do_write, stream); > + ui_file_rewind (current_stream); > } > > /* Return the current MI version. */ > @@ -407,13 +417,14 @@ struct ui_out * > mi_out_new (int mi_version) > { > int flags = 0; > + struct ui_file *new_stream; > > mi_out_data *data = XNEW (mi_out_data); > data->suppress_field_separator = 0; > data->suppress_output = 0; > data->mi_version = mi_version; > - /* FIXME: This code should be using a ``string_file'' and not the > - TUI buffer hack. */ > - data->buffer = mem_fileopen (); > + data->streams = NULL; > + new_stream = mem_fileopen (); > + VEC_safe_push (ui_filep, data->streams, new_stream); > return ui_out_new (&mi_ui_out_impl, data, flags); > } > diff --git a/gdb/testsuite/gdb.mi/mi-logging.exp b/gdb/testsuite/gdb.mi/mi-logging.exp > index d5e4193..27ff1fe 100644 > --- a/gdb/testsuite/gdb.mi/mi-logging.exp > +++ b/gdb/testsuite/gdb.mi/mi-logging.exp > @@ -82,6 +82,20 @@ if [regexp "1001\\^done\[\r\n\]+$mi_log_prompt.*1002\\^running\[\r\n\]+\\*runnin > fail "Redirect log file contents" > } > > +# This triggers a nested mi_ui_out redirection, by executing a python command > +# inside another python command, both of which are run with to_string = True. > +if ![mi_skip_python_tests] { > + mi_gdb_test "-break-insert do_nothing" ".*" > + mi_gdb_test "commands \$bpnum\npython gdb.execute('python gdb.execute(\"help\", True, True)', True, True)\nend" ".*" > + > + mi_send_resuming_command "exec-continue" "continue" > + > + mi_expect_stop "breakpoint-hit" "do_nothing" ".*" ".*" ".*" {".*" ".*"} "Continue to breakpoint" > + > + # This will crash gdb if redirection is not done properly. > + mi_gdb_test "help" ".*List of classes of commands.*\\^done" "nested redirect" > +} > + > mi_gdb_exit > > remote_file host delete $milogfile > diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp > index 1019ecd..4e067cc 100644 > --- a/gdb/testsuite/lib/gdb.exp > +++ b/gdb/testsuite/lib/gdb.exp > @@ -1603,34 +1603,33 @@ proc skip_d_tests {} { > > # Return a 1 for configurations that do not support Python scripting. > > -proc skip_python_tests {} { > - global gdb_prompt > +proc skip_python_tests_prompt { prompt_re } { > global gdb_py_is_py3k > global gdb_py_is_py24 > > gdb_test_multiple "python print ('test')" "verify python support" { > - -re "not supported.*$gdb_prompt $" { > + -re "not supported.*$prompt_re" { > unsupported "Python support is disabled." > return 1 > } > - -re "$gdb_prompt $" {} > + -re "$prompt_re" {} > } > > set gdb_py_is_py24 0 > gdb_test_multiple "python print (sys.version_info\[0\])" "check if python 3" { > - -re "3.*$gdb_prompt $" { > + -re "3.*$prompt_re" { > set gdb_py_is_py3k 1 > } > - -re ".*$gdb_prompt $" { > + -re ".*$prompt_re" { > set gdb_py_is_py3k 0 > } > } > if { $gdb_py_is_py3k == 0 } { > gdb_test_multiple "python print (sys.version_info\[1\])" "check if python 2.4" { > - -re "\[45\].*$gdb_prompt $" { > + -re "\[45\].*$prompt_re" { > set gdb_py_is_py24 1 > } > - -re ".*$gdb_prompt $" { > + -re ".*$prompt_re" { > set gdb_py_is_py24 0 > } > } > @@ -1639,6 +1638,11 @@ proc skip_python_tests {} { > return 0 > } > > +proc skip_python_tests { } { > + global gdb_prompt > + skip_python_tests_prompt "$gdb_prompt $" > +} > + > # Return a 1 if we should skip shared library tests. > > proc skip_shlib_tests {} { > diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp > index 204f1e8..80a3627 100644 > --- a/gdb/testsuite/lib/mi-support.exp > +++ b/gdb/testsuite/lib/mi-support.exp > @@ -2491,3 +2491,10 @@ proc mi_make_breakpoint_table {bp_list} { > # Assemble the final regexp. > return "BreakpointTable={nr_rows=\"$nr\",nr_cols=\"$nc\",$header,$body}" > } > + > +# Return a 1 for configurations that do not support Python scripting. > + > +proc mi_skip_python_tests {} { > + global mi_gdb_prompt > + skip_python_tests_prompt "$mi_gdb_prompt$" > +}