* [PATCH 0/2] Reimplement redirection for MI @ 2014-07-23 14:26 Adrian Sendroiu 2014-07-23 14:26 ` [PATCH 2/2] mi-out: Implement mi redirection using a stack Adrian Sendroiu ` (2 more replies) 0 siblings, 3 replies; 21+ messages in thread From: Adrian Sendroiu @ 2014-07-23 14:26 UTC (permalink / raw) To: gdb-patches; +Cc: Adrian Sendroiu The current implementation uses a "saved_buffer" field to save the current stream when doing a redirection, which will not function correctly in the case of nested mi_redirect calls. These patches reimplement the redirection using a stack of streams, in a manner similar with the one in the cli interpreter. Adrian Sendroiu (2): cli/cli-logging.c: don't call ui_out_redirect for MI when disabling logging mi-out: Implement mi redirection using a stack. gdb/cli/cli-logging.c | 3 +- gdb/mi/mi-out.c | 75 +++++++++++++++++++++++++++++-------------------- 2 files changed, 46 insertions(+), 32 deletions(-) -- 1.7.9.5 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/2] mi-out: Implement mi redirection using a stack. 2014-07-23 14:26 [PATCH 0/2] Reimplement redirection for MI Adrian Sendroiu @ 2014-07-23 14:26 ` Adrian Sendroiu 2014-07-24 20:34 ` Tom Tromey 2014-07-23 14:37 ` [PATCH 1/2] cli/cli-logging.c: don't call ui_out_redirect for MI when disabling logging Adrian Sendroiu 2015-08-17 18:47 ` [PATCH 0/2] Reimplement redirection for MI Doug Evans 2 siblings, 1 reply; 21+ messages in thread From: Adrian Sendroiu @ 2014-07-23 14:26 UTC (permalink / raw) To: gdb-patches; +Cc: Adrian Sendroiu gdb/ 2014-07-23 Adrian Sendroiu <adrian.sendroiu@freescale.com> * mi/mi-out.c (ui_filep): New typedef. (DEF_VEC_P (ui_filep)): New type. (struct ui_out_data): <buffer> Remove field. <original_buffer> Remove field. <streams>: 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. (mi_redirect): Reimplement using push and pop operations. --- gdb/mi/mi-out.c | 75 ++++++++++++++++++++++++++++++++----------------------- 1 file changed, 44 insertions(+), 31 deletions(-) diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c index 6ec41e6..4452080 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,6 +417,7 @@ 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; @@ -414,6 +425,8 @@ mi_out_new (int mi_version) 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); } -- 1.7.9.5 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] mi-out: Implement mi redirection using a stack. 2014-07-23 14:26 ` [PATCH 2/2] mi-out: Implement mi redirection using a stack Adrian Sendroiu @ 2014-07-24 20:34 ` Tom Tromey 2014-07-25 11:43 ` [PATCH v2 " Adrian Sendroiu 0 siblings, 1 reply; 21+ messages in thread From: Tom Tromey @ 2014-07-24 20:34 UTC (permalink / raw) To: Adrian Sendroiu; +Cc: gdb-patches >>>>> "Adrian" == Adrian Sendroiu <adrian.sendroiu@freescale.com> writes: Adrian> gdb/ Adrian> 2014-07-23 Adrian Sendroiu <adrian.sendroiu@freescale.com> Adrian> * mi/mi-out.c (ui_filep): New typedef. [...] I think it would be good to have some text in the message describing the rationale for this patch. One nit below. Adrian> /* FIXME: This code should be using a ``string_file'' and not the Adrian> TUI buffer hack. */ Not your problem but I wonder what this FIXME comment means. I suspect it's obsolete since as far as I know mem_fileopen does return a "string_file". Adrian> + new_stream = mem_fileopen(); Space before the open paren. The patch is ok with this fixed and with some rationale text. thanks, Tom ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 2/2] mi-out: Implement mi redirection using a stack. 2014-07-24 20:34 ` Tom Tromey @ 2014-07-25 11:43 ` Adrian Sendroiu 2014-07-29 15:16 ` Pedro Alves 0 siblings, 1 reply; 21+ messages in thread From: Adrian Sendroiu @ 2014-07-25 11:43 UTC (permalink / raw) To: gdb-patches, tromey; +Cc: Adrian Sendroiu 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 <adrian.sendroiu@freescale.com> * mi/mi-out.c (ui_filep): New typedef. (DEF_VEC_P (ui_filep)): New type. (struct ui_out_data): <buffer> Remove field. <original_buffer> Remove field. <streams>: 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/mi/mi-out.c | 77 +++++++++++++++++++++++++++++++------------------------ 1 file changed, 44 insertions(+), 33 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); } -- 1.7.9.5 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack. 2014-07-25 11:43 ` [PATCH v2 " Adrian Sendroiu @ 2014-07-29 15:16 ` Pedro Alves 2014-07-30 8:38 ` Adrian Sendroiu 0 siblings, 1 reply; 21+ messages in thread From: Pedro Alves @ 2014-07-29 15:16 UTC (permalink / raw) To: Adrian Sendroiu, gdb-patches, tromey I'm guessing we can trigger this by using "save breakpoints" while logging is enabled, like gdb.base/ui-redirect.exp ? I think it'd be very good if a test to the testsuite was added. Sounds like gdb.mi/mi-logging.exp would be a good place? -- Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack. 2014-07-29 15:16 ` Pedro Alves @ 2014-07-30 8:38 ` Adrian Sendroiu 2014-07-30 12:05 ` Pedro Alves 0 siblings, 1 reply; 21+ messages in thread From: Adrian Sendroiu @ 2014-07-30 8:38 UTC (permalink / raw) To: Pedro Alves, gdb-patches, tromey > -----Original Message----- > From: Pedro Alves [mailto:palves@redhat.com] > Sent: Tuesday, July 29, 2014 5:55 PM > To: Sendroiu Adrian-B46904; gdb-patches@sourceware.org; > tromey@redhat.com > Subject: Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a > stack. > > I'm guessing we can trigger this by using "save breakpoints" while > logging is enabled, like gdb.base/ui-redirect.exp ? > I think it'd be very good if a test to the testsuite was added. > Sounds like gdb.mi/mi-logging.exp would be a good place? > > -- > Thanks, > Pedro Alves This won't trigger the bug, because the logging code doesn't call ui_out_redirect if the interpreter is MI. The way I caught it was through some python script that executes commands and catches their output into a string. For example, if you have gdb.execute("break main", False, True) The call sequence will be something like: execute_command_to_string ui_out_redirect execute_command ... mi_breakpoint_created ui_out_redirect Then, after executing this, the mi_uiout->data->buffer will incorrectly point to a freed ui_file structure, and any subsequent command will overwrite the pointers inside this ui_file with random data, causing a crash. Do you have any suggestions on how to make a test case from this scenario? Adrian ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack. 2014-07-30 8:38 ` Adrian Sendroiu @ 2014-07-30 12:05 ` Pedro Alves 2014-07-31 16:23 ` Adrian Sendroiu 0 siblings, 1 reply; 21+ messages in thread From: Pedro Alves @ 2014-07-30 12:05 UTC (permalink / raw) To: Adrian Sendroiu, gdb-patches, tromey On 07/30/2014 09:37 AM, Adrian Sendroiu wrote: >> -----Original Message----- >> From: Pedro Alves [mailto:palves@redhat.com] >> Sent: Tuesday, July 29, 2014 5:55 PM >> To: Sendroiu Adrian-B46904; gdb-patches@sourceware.org; >> tromey@redhat.com >> Subject: Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a >> stack. >> >> I'm guessing we can trigger this by using "save breakpoints" while >> logging is enabled, like gdb.base/ui-redirect.exp ? >> I think it'd be very good if a test to the testsuite was added. >> Sounds like gdb.mi/mi-logging.exp would be a good place? >> >> -- >> Thanks, >> Pedro Alves > > This won't trigger the bug, because the logging code doesn't call ui_out_redirect if the interpreter is MI. The way I caught it was through some python script that executes commands and catches their output into a string. For example, if you have > > gdb.execute("break main", False, True) > > The call sequence will be something like: > execute_command_to_string > ui_out_redirect > execute_command > ... > mi_breakpoint_created > ui_out_redirect > > Then, after executing this, the mi_uiout->data->buffer will incorrectly point to a freed ui_file structure, and any subsequent command will overwrite the pointers inside this ui_file with random data, causing a crash. > > Do you have any suggestions on how to make a test case from this scenario? I'm not sure what specific suggestion you're looking after. :-) Sound like you'd add a test that does that exactly ? You'd either base on, or add to mi-logging.exp, and do something like: mi_gdb_test "python gdb.execute("break main", False, True)" ... mi_gdb_test <some MI command that causes a crash> ... (and skip the test if skip_python_tests is true) Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack. 2014-07-30 12:05 ` Pedro Alves @ 2014-07-31 16:23 ` Adrian Sendroiu 2014-07-31 17:11 ` Pedro Alves 0 siblings, 1 reply; 21+ messages in thread From: Adrian Sendroiu @ 2014-07-31 16:23 UTC (permalink / raw) To: palves, tromey, gdb-patches I implemented the test case. A couple of issues I encountered: - skip_python_tests doesn't work with MI, because it searches for $gdb_prompt. I implemented another one called mi_skip_python_tests which uses $mi_gdb_prompt. - executing "python gdb.execute("break main", ... ) directly doesn't work, because "python" is a cli command, and before running it the current_uiout will be temporarily changed to cli_uiout, so the first redirection won't be done on mi_uiout. To bypass this I implemented a small workaround: set a breakpoint, then set the python command to be executed when the breakpoint hits. - when running the test on a gdb without the redirection fix, the test will be reported as UNRESOLVED, because gdb crashes. --- 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 <adrian.sendroiu@freescale.com> * mi/mi-out.c (ui_filep): New typedef. (DEF_VEC_P (ui_filep)): New type. (struct ui_out_data): <buffer> Remove field. <original_buffer> Remove field. <streams>: 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-07-30 Adrian Sendroiu <adrian.sendroiu@freescale.com> * gdb.mi/mi-logging.exp: Add test for nested mi redirection. * 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/mi-support.exp | 37 +++++++++++++++++ 3 files changed, 95 insertions(+), 33 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..063ea97 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 disabling a breakpoint +# inside a python command that has to_string = True. +if ![mi_skip_python_tests] { + mi_gdb_test "-break-insert do_nothing" ".*" + mi_gdb_test "-break-commands 2 \"python gdb.execute('disable 2', True, True)\"" ".*" + + mi_gdb_test "-exec-continue" ".*" + + set s [string repeat "A" 100] + + # This will crash gdb if redirection is not done properly. + mi_gdb_test "echo $s" ".*\\^done" "mi nested redirect" +} + mi_gdb_exit remote_file host delete $milogfile diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp index 204f1e8..23a5b80 100644 --- a/gdb/testsuite/lib/mi-support.exp +++ b/gdb/testsuite/lib/mi-support.exp @@ -2491,3 +2491,40 @@ proc mi_make_breakpoint_table {bp_list} { # Assemble the final regexp. return "BreakpointTable={nr_rows=\"$nr\",nr_cols=\"$nc\",$header,$body}" } + +proc mi_skip_python_tests {} { + global mi_gdb_prompt + global gdb_py_is_py3k + global gdb_py_is_py24 + + gdb_test_multiple "python print ('test')" "verify python support" { + -re "not supported.*$mi_gdb_prompt$" { + unsupported "Python support is disabled." + return 1 + } + -re "$mi_gdb_prompt$" {} + } + + set gdb_py_is_py24 0 + gdb_test_multiple "python print (sys.version_info\[0\])" "check if python 3" { + -re "3.*$mi_gdb_prompt$" { + set gdb_py_is_py3k 1 + } + -re ".*$mi_gdb_prompt$" { + 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\].*$mi_gdb_prompt$" { + set gdb_py_is_py24 1 + } + -re ".*$mi_gdb_prompt$" { + set gdb_py_is_py24 0 + } + } + } + + return 0 +} -- 1.7.9.5 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack. 2014-07-31 16:23 ` Adrian Sendroiu @ 2014-07-31 17:11 ` Pedro Alves 2014-08-05 13:54 ` Adrian Sendroiu 0 siblings, 1 reply; 21+ messages in thread From: Pedro Alves @ 2014-07-31 17:11 UTC (permalink / raw) To: Adrian Sendroiu, tromey, gdb-patches Awesome! Thanks for doing this. This is close. Some notes on the technicalities below. On 07/31/2014 04:08 PM, Adrian Sendroiu wrote: > +# This triggers a nested mi_ui_out redirection, by disabling a breakpoint > +# inside a python command that has to_string = True. > +if ![mi_skip_python_tests] { > + mi_gdb_test "-break-insert do_nothing" ".*" > + mi_gdb_test "-break-commands 2 \"python gdb.execute('disable 2', True, True)\"" ".*" It's better to use $bpnum instead of hardcoding 2, as otherwise if someone adds a test that adds another breakpoint before this, this test stops being effective, silently. > + > + mi_gdb_test "-exec-continue" ".*" This should use mi_send_resuming_command/mi_expect_stop or mi_execute_to, so that the test works when the whole MI testsuite is run in async mode. > + > + set s [string repeat "A" 100] > + > + # This will crash gdb if redirection is not done properly. > + mi_gdb_test "echo $s" ".*\\^done" "mi nested redirect" In addition to that, it'd be good to confirm the breakpoint did end up disabled, which likewise confirms the breakpoint command was set on the breakpoint we wanted. (might not need the "echo" if that itself already causes a crash.) > +proc mi_skip_python_tests {} { > + global mi_gdb_prompt > + global gdb_py_is_py3k > + global gdb_py_is_py24 > + > + gdb_test_multiple "python print ('test')" "verify python support" { > + -re "not supported.*$mi_gdb_prompt$" { > + unsupported "Python support is disabled." > + return 1 > + } > + -re "$mi_gdb_prompt$" {} > + } > + > + set gdb_py_is_py24 0 > + gdb_test_multiple "python print (sys.version_info\[0\])" "check if python 3" { > + -re "3.*$mi_gdb_prompt$" { > + set gdb_py_is_py3k 1 > + } > + -re ".*$mi_gdb_prompt$" { > + 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\].*$mi_gdb_prompt$" { > + set gdb_py_is_py24 1 > + } > + -re ".*$mi_gdb_prompt$" { > + set gdb_py_is_py24 0 > + } > + } > + } > + > + return 0 > +} I think that we can avoid this duplication by renaming skip_python_tests, adding it a prompt_re parameter, and using that instead of $gdb_prompt. Something like: proc skip_python_tests {} { skip_python_tests_prompt "$gdb_prompt $" } proc mi_skip_python_tests { skip_python_tests_prompt "$mi_gdb_prompt$" } Did you try that? Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack. 2014-07-31 17:11 ` Pedro Alves @ 2014-08-05 13:54 ` Adrian Sendroiu 2014-08-28 11:33 ` Adrian Sendroiu ` (2 more replies) 0 siblings, 3 replies; 21+ messages in thread From: Adrian Sendroiu @ 2014-08-05 13:54 UTC (permalink / raw) To: Pedro Alves, tromey, gdb-patches > It's better to use $bpnum instead of hardcoding 2, as otherwise if someone adds a > test that adds another breakpoint before this, this test stops being > effective, silently. done >> + mi_gdb_test "-exec-continue" ".*" > > This should use mi_send_resuming_command/mi_expect_stop > or mi_execute_to, so that the test works when the whole MI > testsuite is run in async mode. done. Although I ran into another problem here, because mi_expect_stop expects a message that looks like *stopped + prompt, while in my case it was something like *stopped + =breakpoint-modified + prompt. I solved this by doing an even simpler test case: just executing two python commands nested inside one another, like "python gdb.execute('python gdb.execute(...". > I think that we can avoid this duplication by renaming > skip_python_tests, adding it a prompt_re parameter, and > using that instead of $gdb_prompt. Something like: > > proc skip_python_tests {} { > skip_python_tests_prompt "$gdb_prompt $" > } > > proc mi_skip_python_tests { > skip_python_tests_prompt "$mi_gdb_prompt$" > } > > Did you try that? done --- 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 <adrian.sendroiu@freescale.com> * mi/mi-out.c (ui_filep): New typedef. (DEF_VEC_P (ui_filep)): New type. (struct ui_out_data): <buffer> Remove field. <original_buffer> Remove field. <streams>: 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 <adrian.sendroiu@freescale.com> * 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..bd884d3 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" ".*" "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 8cb98ae..8bb2d23 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$" +} -- 1.7.9.5 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack. 2014-08-05 13:54 ` Adrian Sendroiu @ 2014-08-28 11:33 ` Adrian Sendroiu 2014-09-05 16:06 ` Pedro Alves 2014-09-08 13:19 ` Pedro Alves 2 siblings, 0 replies; 21+ messages in thread From: Adrian Sendroiu @ 2014-08-28 11:33 UTC (permalink / raw) To: Pedro Alves, tromey, gdb-patches; +Cc: adrian.sendroiu ping On 08/05/2014 04:55 PM, Adrian Sendroiu wrote: >> It's better to use $bpnum instead of hardcoding 2, as otherwise if someone adds a >> test that adds another breakpoint before this, this test stops being >> effective, silently. > > done > >>> + mi_gdb_test "-exec-continue" ".*" >> >> This should use mi_send_resuming_command/mi_expect_stop >> or mi_execute_to, so that the test works when the whole MI >> testsuite is run in async mode. > > done. Although I ran into another problem here, because mi_expect_stop expects a > message that looks like *stopped + prompt, while in my case it was something like > *stopped + =breakpoint-modified + prompt. I solved this by doing an even simpler > test case: just executing two python commands nested inside one another, like > "python gdb.execute('python gdb.execute(...". > > >> I think that we can avoid this duplication by renaming >> skip_python_tests, adding it a prompt_re parameter, and >> using that instead of $gdb_prompt. Something like: >> >> proc skip_python_tests {} { >> skip_python_tests_prompt "$gdb_prompt $" >> } >> >> proc mi_skip_python_tests { >> skip_python_tests_prompt "$mi_gdb_prompt$" >> } >> >> Did you try that? > > done > > --- 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 <adrian.sendroiu@freescale.com> > > * mi/mi-out.c (ui_filep): New typedef. > (DEF_VEC_P (ui_filep)): New type. > (struct ui_out_data): <buffer> Remove field. > <original_buffer> Remove field. > <streams>: 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 <adrian.sendroiu@freescale.com> > > * 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..bd884d3 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" ".*" "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 8cb98ae..8bb2d23 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$" > +} > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack. 2014-08-05 13:54 ` Adrian Sendroiu 2014-08-28 11:33 ` Adrian Sendroiu @ 2014-09-05 16:06 ` Pedro Alves 2014-09-07 15:45 ` Adrian Sendroiu 2014-09-08 13:19 ` Pedro Alves 2 siblings, 1 reply; 21+ messages in thread From: Pedro Alves @ 2014-09-05 16:06 UTC (permalink / raw) To: Adrian Sendroiu; +Cc: tromey, gdb-patches Hi Adrian, On 08/05/2014 02:55 PM, Adrian Sendroiu wrote: >> It's better to use $bpnum instead of hardcoding 2, as otherwise if someone adds a >> test that adds another breakpoint before this, this test stops being >> effective, silently. > > done > >>> + mi_gdb_test "-exec-continue" ".*" >> >> This should use mi_send_resuming_command/mi_expect_stop >> or mi_execute_to, so that the test works when the whole MI >> testsuite is run in async mode. > > done. Although I ran into another problem here, because mi_expect_stop expects a > message that looks like *stopped + prompt, while in my case it was something like > *stopped + =breakpoint-modified + prompt. I solved this by doing an even simpler > test case: just executing two python commands nested inside one another, like > "python gdb.execute('python gdb.execute(...". > > >> I think that we can avoid this duplication by renaming >> skip_python_tests, adding it a prompt_re parameter, and >> using that instead of $gdb_prompt. Something like: >> >> proc skip_python_tests {} { >> skip_python_tests_prompt "$gdb_prompt $" >> } >> >> proc mi_skip_python_tests { >> skip_python_tests_prompt "$mi_gdb_prompt$" >> } >> >> Did you try that? > > done > Thanks. I'm getting these with this patch: Running /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/mi-logging.exp ... PASS: gdb.mi/mi-logging.exp: breakpoint at main PASS: gdb.mi/mi-logging.exp: mi runto main PASS: gdb.mi/mi-logging.exp: logging on PASS: gdb.mi/mi-logging.exp: logged step PASS: gdb.mi/mi-logging.exp: logged next ERROR: Got interactive prompt. UNRESOLVED: gdb.mi/mi-logging.exp: logging off PASS: gdb.mi/mi-logging.exp: Log file contents ERROR: Got interactive prompt. UNRESOLVED: gdb.mi/mi-logging.exp: redirect logging on ERROR: Got interactive prompt. UNRESOLVED: gdb.mi/mi-logging.exp: redirect logging off FAIL: gdb.mi/mi-logging.exp: Redirect log file contents FAIL: gdb.mi/mi-logging.exp: verify python support (GDB internal error) ERROR: Could not resync from internal error (timeout) UNRESOLVED: gdb.mi/mi-logging.exp: check if python 3 (got interactive prompt) ERROR: tcl error sourcing /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/mi-logging.exp. ERROR: can't read "gdb_py_is_py3k": no such variable while executing ... gdb.log shows: Expecting: ^(-gdb-set logging off[ ]+)?(.*[ ]+[(]gdb[)] [ ]*) -gdb-set logging off ^done~"/home/pedro/gdb/mygit/src/gdb/mi/mi-out.c:398: internal-error: VEC_ui_filep_last: Assertion `last' failed.\nA problem internal to GDB has been detected,\nfurther deb ugging may prove unreliable.\nQuit this debugging session? " ~"(y or n) " ERROR: Got interactive prompt. UNRESOLVED: gdb.mi/mi-logging.exp: logging off PASS: gdb.mi/mi-logging.exp: Log file contents Expecting: ^(-gdb-set logging redirect on[ ]+)?(.*[ ]+[(]gdb[)] [ ]*) Any idea what went wrong ? Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack. 2014-09-05 16:06 ` Pedro Alves @ 2014-09-07 15:45 ` Adrian Sendroiu 0 siblings, 0 replies; 21+ messages in thread From: Adrian Sendroiu @ 2014-09-07 15:45 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches, Tom Tromey, Adrian Sendroiu There's also the 1/2 patch from the series that needs to be applied before this. https://sourceware.org/ml/gdb-patches/2014-07/msg00574.html Adrian ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack. 2014-08-05 13:54 ` Adrian Sendroiu 2014-08-28 11:33 ` Adrian Sendroiu 2014-09-05 16:06 ` Pedro Alves @ 2014-09-08 13:19 ` Pedro Alves 2014-09-08 18:59 ` Sergio Durigan Junior 2014-09-09 14:03 ` Adrian Sendroiu 2 siblings, 2 replies; 21+ messages in thread From: Pedro Alves @ 2014-09-08 13:19 UTC (permalink / raw) To: Adrian Sendroiu; +Cc: tromey, gdb-patches On 08/05/2014 02:55 PM, Adrian Sendroiu wrote: > There's also the 1/2 patch from the series that needs to be applied > before this. > > https://sourceware.org/ml/gdb-patches/2014-07/msg00574.html Ah. Silly me, completely missed that. The test indeed passes cleanly for me with that applied. > + mi_expect_stop "breakpoint-hit" "do_nothing" ".*" ".*" ".*" {".*" ".*"} "Continue to breakpoint" > + > + # This will crash gdb if redirection is not done properly. > + mi_gdb_test "help" ".*" "nested redirect" 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. Thanks! Pedro Alves ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack. 2014-09-08 13:19 ` Pedro Alves @ 2014-09-08 18:59 ` Sergio Durigan Junior 2014-09-09 14:03 ` Adrian Sendroiu 1 sibling, 0 replies; 21+ messages in thread From: Sergio Durigan Junior @ 2014-09-08 18:59 UTC (permalink / raw) To: Pedro Alves; +Cc: Adrian Sendroiu, tromey, gdb-patches On Monday, September 08 2014, Pedro Alves wrote: >> + mi_expect_stop "breakpoint-hit" "do_nothing" ".*" ".*" ".*" {".*" ".*"} "Continue to breakpoint" >> + >> + # This will crash gdb if redirection is not done properly. >> + mi_gdb_test "help" ".*" "nested redirect" > > 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" I've been seeing some cases like this in the list (i.e., writing tests using ".*" a lot). Maybe this is a good thing to mention in our wiki... -- Sergio GPG key ID: 0x65FC5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack. 2014-09-08 13:19 ` Pedro Alves 2014-09-08 18:59 ` Sergio Durigan Junior @ 2014-09-09 14:03 ` Adrian Sendroiu 2014-09-26 9:25 ` Adrian Sendroiu 1 sibling, 1 reply; 21+ messages in thread From: Adrian Sendroiu @ 2014-09-09 14:03 UTC (permalink / raw) To: Pedro Alves; +Cc: tromey, gdb-patches, Adrian Sendroiu > 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 <adrian.sendroiu@freescale.com> * mi/mi-out.c (ui_filep): New typedef. (DEF_VEC_P (ui_filep)): New type. (struct ui_out_data): <buffer> Remove field. <original_buffer> Remove field. <streams>: 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 <adrian.sendroiu@freescale.com> * 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$" +} -- 1.8.5.5 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack. 2014-09-09 14:03 ` Adrian Sendroiu @ 2014-09-26 9:25 ` Adrian Sendroiu 2014-09-26 12:51 ` Pedro Alves 0 siblings, 1 reply; 21+ messages in thread From: Adrian Sendroiu @ 2014-09-26 9:25 UTC (permalink / raw) To: Pedro Alves; +Cc: tromey, gdb-patches ping Adrian Sendroiu <molecula2788@gmail.com> 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 <adrian.sendroiu@freescale.com> > > * mi/mi-out.c (ui_filep): New typedef. > (DEF_VEC_P (ui_filep)): New type. > (struct ui_out_data): <buffer> Remove field. > <original_buffer> Remove field. > <streams>: 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 <adrian.sendroiu@freescale.com> > > * 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$" > +} ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/2] mi-out: Implement mi redirection using a stack. 2014-09-26 9:25 ` Adrian Sendroiu @ 2014-09-26 12:51 ` Pedro Alves 0 siblings, 0 replies; 21+ messages in thread From: Pedro Alves @ 2014-09-26 12:51 UTC (permalink / raw) To: Adrian Sendroiu; +Cc: gdb-patches On 09/26/2014 10:24 AM, Adrian Sendroiu wrote: > Adrian Sendroiu <molecula2788@gmail.com> 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" OK. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/2] cli/cli-logging.c: don't call ui_out_redirect for MI when disabling logging 2014-07-23 14:26 [PATCH 0/2] Reimplement redirection for MI Adrian Sendroiu 2014-07-23 14:26 ` [PATCH 2/2] mi-out: Implement mi redirection using a stack Adrian Sendroiu @ 2014-07-23 14:37 ` Adrian Sendroiu 2014-07-24 18:19 ` Tom Tromey 2015-08-17 18:47 ` [PATCH 0/2] Reimplement redirection for MI Doug Evans 2 siblings, 1 reply; 21+ messages in thread From: Adrian Sendroiu @ 2014-07-23 14:37 UTC (permalink / raw) To: gdb-patches; +Cc: Adrian Sendroiu When logging is set up, the code skips the call to ui_out_redirect if the current interpreter is MI, but still calls it unconditionally when logging is turned off. Since the number of "redirect"/"unredirect" calls must match, this patch ensures that the ui_out_redirect call is also skipped when logging is turned off. gdb/ 2014-07-23 Adrian Sendroiu <adrian.sendroiu@freescale.com> * cli/cli-logging.c (pop_output_files): Don't call ui_out_redirect if the current interpreter is MI. --- gdb/cli/cli-logging.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gdb/cli/cli-logging.c b/gdb/cli/cli-logging.c index aae0039..e51bb95 100644 --- a/gdb/cli/cli-logging.c +++ b/gdb/cli/cli-logging.c @@ -180,7 +180,8 @@ pop_output_files (void) saved_output.targ = NULL; saved_output.targerr = NULL; - ui_out_redirect (current_uiout, NULL); + if (!ui_out_is_mi_like_p (current_uiout)) + ui_out_redirect (current_uiout, NULL); } /* This is a helper for the `set logging' command. */ -- 1.7.9.5 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] cli/cli-logging.c: don't call ui_out_redirect for MI when disabling logging 2014-07-23 14:37 ` [PATCH 1/2] cli/cli-logging.c: don't call ui_out_redirect for MI when disabling logging Adrian Sendroiu @ 2014-07-24 18:19 ` Tom Tromey 0 siblings, 0 replies; 21+ messages in thread From: Tom Tromey @ 2014-07-24 18:19 UTC (permalink / raw) To: Adrian Sendroiu; +Cc: gdb-patches >>>>> "Adrian" == Adrian Sendroiu <adrian.sendroiu@freescale.com> writes: Adrian> 2014-07-23 Adrian Sendroiu <adrian.sendroiu@freescale.com> Adrian> * cli/cli-logging.c (pop_output_files): Don't call ui_out_redirect Adrian> if the current interpreter is MI. Thanks, this is ok. Tom ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/2] Reimplement redirection for MI 2014-07-23 14:26 [PATCH 0/2] Reimplement redirection for MI Adrian Sendroiu 2014-07-23 14:26 ` [PATCH 2/2] mi-out: Implement mi redirection using a stack Adrian Sendroiu 2014-07-23 14:37 ` [PATCH 1/2] cli/cli-logging.c: don't call ui_out_redirect for MI when disabling logging Adrian Sendroiu @ 2015-08-17 18:47 ` Doug Evans 2 siblings, 0 replies; 21+ messages in thread From: Doug Evans @ 2015-08-17 18:47 UTC (permalink / raw) To: Adrian Sendroiu; +Cc: gdb-patches On Wed, Jul 23, 2014 at 7:26 AM, Adrian Sendroiu <adrian.sendroiu@freescale.com> wrote: > The current implementation uses a "saved_buffer" field to save the current > stream when doing a redirection, which will not function correctly in the case > of nested mi_redirect calls. These patches reimplement the redirection using a > stack of streams, in a manner similar with the one in the cli interpreter. > > Adrian Sendroiu (2): > cli/cli-logging.c: don't call ui_out_redirect for MI when disabling > logging > mi-out: Implement mi redirection using a stack. > > gdb/cli/cli-logging.c | 3 +- > gdb/mi/mi-out.c | 75 +++++++++++++++++++++++++++++-------------------- > 2 files changed, 46 insertions(+), 32 deletions(-) Hi. Filing for reference sake: This is pr 18833. https://sourceware.org/bugzilla/show_bug.cgi?id=18833 Also for reference sake: threads for 1/2 and 2/2 are here: https://sourceware.org/ml/gdb-patches/2014-07/msg00574.html https://sourceware.org/ml/gdb-patches/2014-07/msg00573.html 2/2 v2: https://sourceware.org/ml/gdb-patches/2014-07/msg00665.html ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2015-08-17 18:47 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-07-23 14:26 [PATCH 0/2] Reimplement redirection for MI Adrian Sendroiu 2014-07-23 14:26 ` [PATCH 2/2] mi-out: Implement mi redirection using a stack Adrian Sendroiu 2014-07-24 20:34 ` Tom Tromey 2014-07-25 11:43 ` [PATCH v2 " Adrian Sendroiu 2014-07-29 15:16 ` Pedro Alves 2014-07-30 8:38 ` Adrian Sendroiu 2014-07-30 12:05 ` Pedro Alves 2014-07-31 16:23 ` Adrian Sendroiu 2014-07-31 17:11 ` Pedro Alves 2014-08-05 13:54 ` Adrian Sendroiu 2014-08-28 11:33 ` Adrian Sendroiu 2014-09-05 16:06 ` Pedro Alves 2014-09-07 15:45 ` Adrian Sendroiu 2014-09-08 13:19 ` Pedro Alves 2014-09-08 18:59 ` Sergio Durigan Junior 2014-09-09 14:03 ` Adrian Sendroiu 2014-09-26 9:25 ` Adrian Sendroiu 2014-09-26 12:51 ` Pedro Alves 2014-07-23 14:37 ` [PATCH 1/2] cli/cli-logging.c: don't call ui_out_redirect for MI when disabling logging Adrian Sendroiu 2014-07-24 18:19 ` Tom Tromey 2015-08-17 18:47 ` [PATCH 0/2] Reimplement redirection for MI Doug Evans
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).