From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10191 invoked by alias); 27 Jan 2011 14:15:52 -0000 Received: (qmail 10120 invoked by uid 22791); 27 Jan 2011 14:15:46 -0000 X-SWARE-Spam-Status: No, hits=-6.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 27 Jan 2011 14:15:39 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id p0REFWOR015800 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 27 Jan 2011 09:15:34 -0500 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p0REFW0m032687; Thu, 27 Jan 2011 09:15:32 -0500 Received: from opsy.redhat.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id p0REFVVi001230; Thu, 27 Jan 2011 09:15:31 -0500 Received: by opsy.redhat.com (Postfix, from userid 500) id D1A6A37887A; Thu, 27 Jan 2011 07:15:30 -0700 (MST) From: Tom Tromey To: gdb-patches@sourceware.org Subject: RFC: change MI event channel to use ui-out Date: Thu, 27 Jan 2011 17:36:00 -0000 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-01/txt/msg00519.txt.bz2 This patch changes MI's event channel from a ui-file to a ui-out. The rationale for this comes in a later patch: I wanted to emit some async breakpoint notifications, and it seemed nicer to reuse the existing breakpoint-printing functions, which all use ui-out objects. However, I think this is a reasonable cleanup on its own. Its primary plus is that it makes it more difficult to emit incorrect output. E.g., right now, if a library name contains a double quote, I think gdb will print the wrong this, but after this patch, the output will be correct.q This change required a little hackery in the MI ui-out object itself. I changed it to format top-level lists slightly differently in the event channel case. Built and regtested on x86-64 (compile farm). Let me know what you think. In the absence of comments I will commit this after a decent interval. Tom 2011-01-26 Tom Tromey * mi/mi-out.h (mi_out_new): Update. * mi/mi-out.c (struct ui_out_data) : New fields. (mi_open): Handle new fields. (mi_close): Likewise. (mi_out_new): Add 'out' argument. Update for new fields. * mi/mi-main.c (mi_execute_command): Update. (mi_load_progress): Update. * mi/mi-interp.c (mi_interpreter_init): Update. (mi_new_thread): Update. (mi_thread_exit): Likewise. (mi_inferior_added): Likewise. (mi_inferior_appeared): Likewise. (mi_inferior_exit): Likewise. (mi_inferior_removed): Likewise. (mi_solib_loaded): Likewise. (mi_solib_unloaded): Likewise. (report_initial_inferior): Likewise. (_initialize_mi_interp): Likewise. * mi/mi-common.h (struct mi_interp) : New a ui_out*. diff --git a/gdb/mi/mi-common.h b/gdb/mi/mi-common.h index e3aab7d..6df1678 100644 --- a/gdb/mi/mi-common.h +++ b/gdb/mi/mi-common.h @@ -50,7 +50,7 @@ struct mi_interp struct ui_file *err; struct ui_file *log; struct ui_file *targ; - struct ui_file *event_channel; + struct ui_out *event_channel; /* This is the interpreter for the mi... */ struct interp *mi2_interp; diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c index 23c60f6..ea69c75 100644 --- a/gdb/mi/mi-interp.c +++ b/gdb/mi/mi-interp.c @@ -84,7 +84,8 @@ mi_interpreter_init (int top_level) mi->err = mi_console_file_new (raw_stdout, "&", '"'); mi->log = mi->err; mi->targ = mi_console_file_new (raw_stdout, "@", '"'); - mi->event_channel = mi_console_file_new (raw_stdout, "=", 0); + mi->event_channel = mi_out_new (3 /* This doesn't actually matter. */, + mi_console_file_new (raw_stdout, "=", 0)); if (top_level) { @@ -301,13 +302,15 @@ mi_new_thread (struct thread_info *t) { struct mi_interp *mi = top_level_interpreter_data (); struct inferior *inf = find_inferior_pid (ptid_get_pid (t->ptid)); + struct cleanup *cleanup; gdb_assert (inf); - fprintf_unfiltered (mi->event_channel, - "thread-created,id=\"%d\",group-id=\"i%d\"", - t->num, inf->num); - gdb_flush (mi->event_channel); + cleanup = make_cleanup_ui_out_list_begin_end (mi->event_channel, + "thread_created"); + ui_out_field_int (mi->event_channel, "id", t->num); + ui_out_field_fmt (mi->event_channel, "group-id", "i%d", inf->num); + do_cleanups (cleanup); } static void @@ -315,6 +318,7 @@ mi_thread_exit (struct thread_info *t, int silent) { struct mi_interp *mi; struct inferior *inf; + struct cleanup *cleanup; if (silent) return; @@ -323,57 +327,64 @@ mi_thread_exit (struct thread_info *t, int silent) mi = top_level_interpreter_data (); target_terminal_ours (); - fprintf_unfiltered (mi->event_channel, - "thread-exited,id=\"%d\",group-id=\"i%d\"", - t->num, inf->num); - gdb_flush (mi->event_channel); + cleanup = make_cleanup_ui_out_list_begin_end (mi->event_channel, + "thread-exited"); + ui_out_field_int (mi->event_channel, "id", t->num); + ui_out_field_fmt (mi->event_channel, "group-id", "i%d", inf->num); + do_cleanups (cleanup); } static void mi_inferior_added (struct inferior *inf) { struct mi_interp *mi = top_level_interpreter_data (); + struct cleanup *cleanup; target_terminal_ours (); - fprintf_unfiltered (mi->event_channel, - "thread-group-added,id=\"i%d\"", - inf->num); - gdb_flush (mi->event_channel); + cleanup = make_cleanup_ui_out_list_begin_end (mi->event_channel, + "thread-group-added"); + ui_out_field_fmt (mi->event_channel, "id", "i%d", inf->num); + do_cleanups (cleanup); } static void mi_inferior_appeared (struct inferior *inf) { struct mi_interp *mi = top_level_interpreter_data (); + struct cleanup *cleanup; target_terminal_ours (); - fprintf_unfiltered (mi->event_channel, - "thread-group-started,id=\"i%d\",pid=\"%d\"", - inf->num, inf->pid); - gdb_flush (mi->event_channel); + cleanup = make_cleanup_ui_out_list_begin_end (mi->event_channel, + "thread-group-started"); + ui_out_field_fmt (mi->event_channel, "id", "i%d", inf->num); + ui_out_field_fmt (mi->event_channel, "pid", "%d", inf->pid); + do_cleanups (cleanup); } static void mi_inferior_exit (struct inferior *inf) { struct mi_interp *mi = top_level_interpreter_data (); + struct cleanup *cleanup; target_terminal_ours (); - fprintf_unfiltered (mi->event_channel, "thread-group-exited,id=\"i%d\"", - inf->num); - gdb_flush (mi->event_channel); + cleanup = make_cleanup_ui_out_list_begin_end (mi->event_channel, + "thread-group-exited"); + ui_out_field_fmt (mi->event_channel, "id", "i%d", inf->num); + do_cleanups (cleanup); } static void mi_inferior_removed (struct inferior *inf) { struct mi_interp *mi = top_level_interpreter_data (); + struct cleanup *cleanup; target_terminal_ours (); - fprintf_unfiltered (mi->event_channel, - "thread-group-removed,id=\"i%d\"", - inf->num); - gdb_flush (mi->event_channel); + cleanup = make_cleanup_ui_out_list_begin_end (mi->event_channel, + "thread-group-removed"); + ui_out_field_fmt (mi->event_channel, "id", "i%d", inf->num); + do_cleanups (cleanup); } static void @@ -540,62 +551,70 @@ static void mi_solib_loaded (struct so_list *solib) { struct mi_interp *mi = top_level_interpreter_data (); + struct cleanup *cleanup; target_terminal_ours (); - if (gdbarch_has_global_solist (target_gdbarch)) - fprintf_unfiltered (mi->event_channel, - "library-loaded,id=\"%s\",target-name=\"%s\"," - "host-name=\"%s\",symbols-loaded=\"%d\"", - solib->so_original_name, solib->so_original_name, - solib->so_name, solib->symbols_loaded); - else - fprintf_unfiltered (mi->event_channel, - "library-loaded,id=\"%s\",target-name=\"%s\"," - "host-name=\"%s\",symbols-loaded=\"%d\"," - "thread-group=\"i%d\"", - solib->so_original_name, solib->so_original_name, - solib->so_name, solib->symbols_loaded, - current_inferior ()->num); - - gdb_flush (mi->event_channel); + + cleanup = make_cleanup_ui_out_list_begin_end (mi->event_channel, + "library-loaded"); + + ui_out_field_string (mi->event_channel, "id", + solib->so_original_name); + ui_out_field_string (mi->event_channel, "target-name", + solib->so_original_name); + ui_out_field_string (mi->event_channel, "host-name", + solib->so_name); + ui_out_field_int (mi->event_channel, "symbols-loaded", + solib->symbols_loaded); + + if (! gdbarch_has_global_solist (target_gdbarch)) + ui_out_field_fmt (mi->event_channel, "thread-group", "i%d", + current_inferior ()->num); + + do_cleanups (cleanup); } static void mi_solib_unloaded (struct so_list *solib) { struct mi_interp *mi = top_level_interpreter_data (); + struct cleanup *cleanup; target_terminal_ours (); - if (gdbarch_has_global_solist (target_gdbarch)) - fprintf_unfiltered (mi->event_channel, - "library-unloaded,id=\"%s\",target-name=\"%s\"," - "host-name=\"%s\"", - solib->so_original_name, solib->so_original_name, - solib->so_name); - else - fprintf_unfiltered (mi->event_channel, - "library-unloaded,id=\"%s\",target-name=\"%s\"," - "host-name=\"%s\",thread-group=\"i%d\"", - solib->so_original_name, solib->so_original_name, - solib->so_name, current_inferior ()->num); - gdb_flush (mi->event_channel); + cleanup = make_cleanup_ui_out_list_begin_end (mi->event_channel, + "library-unloaded"); + ui_out_field_string (mi->event_channel, "id", + solib->so_original_name); + ui_out_field_string (mi->event_channel, "target-name", + solib->so_original_name); + ui_out_field_string (mi->event_channel, "host-name", + solib->so_name); + + if (! gdbarch_has_global_solist (target_gdbarch)) + ui_out_field_fmt (mi->event_channel, "thread-group", "i%d", + current_inferior ()->num); + + do_cleanups (cleanup); } static int report_initial_inferior (struct inferior *inf, void *closure) { - /* This function is called from mi_intepreter_init, and since + /* This function is called from mi_interpreter_init, and since mi_inferior_added assumes that inferior is fully initialized and top_level_interpreter_data is set, we cannot call it here. */ struct mi_interp *mi = closure; + struct cleanup *cleanup; target_terminal_ours (); - fprintf_unfiltered (mi->event_channel, - "thread-group-added,id=\"i%d\"", - inf->num); - gdb_flush (mi->event_channel); + + cleanup = make_cleanup_ui_out_list_begin_end (mi->event_channel, + "thread-group-added"); + ui_out_field_fmt (mi->event_channel, "added", "i%d", inf->num); + do_cleanups (cleanup); + return 0; } @@ -614,11 +633,11 @@ _initialize_mi_interp (void) }; /* The various interpreter levels. */ - interp_add (interp_new (INTERP_MI1, NULL, mi_out_new (1), &procs)); - interp_add (interp_new (INTERP_MI2, NULL, mi_out_new (2), &procs)); - interp_add (interp_new (INTERP_MI3, NULL, mi_out_new (3), &procs)); + interp_add (interp_new (INTERP_MI1, NULL, mi_out_new (1, NULL), &procs)); + interp_add (interp_new (INTERP_MI2, NULL, mi_out_new (2, NULL), &procs)); + interp_add (interp_new (INTERP_MI3, NULL, mi_out_new (3, NULL), &procs)); /* "mi" selects the most recent released version. "mi2" was released as part of GDB 6.0. */ - interp_add (interp_new (INTERP_MI, NULL, mi_out_new (2), &procs)); + interp_add (interp_new (INTERP_MI, NULL, mi_out_new (2, NULL), &procs)); } diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 1f1b712..d9e988c 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -2004,12 +2004,15 @@ mi_execute_command (char *cmd, int from_tty) if (report_change) { struct thread_info *ti = inferior_thread (); + struct cleanup *cleanup; target_terminal_ours (); - fprintf_unfiltered (mi->event_channel, - "thread-selected,id=\"%d\"", - ti->num); - gdb_flush (mi->event_channel); + + cleanup + = make_cleanup_ui_out_list_begin_end (mi->event_channel, + "thread-selected"); + ui_out_field_int (mi->event_channel, "id", ti->num); + do_cleanups (cleanup); } } @@ -2192,11 +2195,11 @@ mi_load_progress (const char *section_name, if (current_interp_named_p (INTERP_MI) || current_interp_named_p (INTERP_MI2)) - uiout = mi_out_new (2); + uiout = mi_out_new (2, NULL); else if (current_interp_named_p (INTERP_MI1)) - uiout = mi_out_new (1); + uiout = mi_out_new (1, NULL); else if (current_interp_named_p (INTERP_MI3)) - uiout = mi_out_new (3); + uiout = mi_out_new (3, NULL); else return; diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c index 9aaeec6..4cd6abc 100644 --- a/gdb/mi/mi-out.c +++ b/gdb/mi/mi-out.c @@ -29,6 +29,12 @@ struct ui_out_data int suppress_field_separator; int suppress_output; int mi_version; + /* True if this ui-out is for event notification. This changes + how some lists are output. */ + int for_events; + /* The number of list-opens we have seen. This is only used when + FOR_EVENTS is true. */ + int list_depth; struct ui_file *buffer; }; typedef struct ui_out_data mi_out_data; @@ -315,18 +321,34 @@ mi_open (struct ui_out *uiout, enum ui_out_type type) { mi_out_data *data = ui_out_data (uiout); + int event_start = 0; - field_separator (uiout); - data->suppress_field_separator = 1; + if (type == ui_out_type_list && data->for_events) + { + ++data->list_depth; + if (data->list_depth == 1) + event_start = 1; + } + + if (!event_start) + { + field_separator (uiout); + data->suppress_field_separator = 1; + } if (name) - fprintf_unfiltered (data->buffer, "%s=", name); + { + fputs_unfiltered (name, data->buffer); + if (!event_start) + fputs_unfiltered ("=", data->buffer); + } switch (type) { case ui_out_type_tuple: fputc_unfiltered ('{', data->buffer); break; case ui_out_type_list: - fputc_unfiltered ('[', data->buffer); + if (!event_start) + fputc_unfiltered ('[', data->buffer); break; default: internal_error (__FILE__, __LINE__, _("bad switch")); @@ -338,6 +360,14 @@ mi_close (struct ui_out *uiout, enum ui_out_type type) { mi_out_data *data = ui_out_data (uiout); + int event_end = 0; + + if (type == ui_out_type_list && data->for_events) + { + --data->list_depth; + if (data->list_depth == 0) + event_end = 1; + } switch (type) { @@ -345,12 +375,16 @@ mi_close (struct ui_out *uiout, fputc_unfiltered ('}', data->buffer); break; case ui_out_type_list: - fputc_unfiltered (']', data->buffer); + if (!event_end) + fputc_unfiltered (']', data->buffer); break; default: internal_error (__FILE__, __LINE__, _("bad switch")); } data->suppress_field_separator = 0; + + if (event_end) + mi_flush (uiout); } /* add a string to the buffer */ @@ -401,10 +435,10 @@ mi_version (struct ui_out *uiout) return data->mi_version; } -/* initalize private members at startup */ +/* Initialize private members at startup. */ struct ui_out * -mi_out_new (int mi_version) +mi_out_new (int mi_version, struct ui_file *out) { int flags = 0; @@ -412,9 +446,14 @@ mi_out_new (int mi_version) data->suppress_field_separator = 0; data->suppress_output = 0; data->mi_version = mi_version; + data->for_events = out != NULL; + data->list_depth = 0; /* FIXME: This code should be using a ``string_file'' and not the TUI buffer hack. */ - data->buffer = mem_fileopen (); + if (out) + data->buffer = out; + else + data->buffer = mem_fileopen (); return ui_out_new (&mi_ui_out_impl, data, flags); } diff --git a/gdb/mi/mi-out.h b/gdb/mi/mi-out.h index 515aefa..d7bf0a6 100644 --- a/gdb/mi/mi-out.h +++ b/gdb/mi/mi-out.h @@ -24,7 +24,7 @@ struct ui_out; struct ui_file; -extern struct ui_out *mi_out_new (int mi_version); +extern struct ui_out *mi_out_new (int mi_version, struct ui_file *stream); extern void mi_out_put (struct ui_out *uiout, struct ui_file *stream); extern void mi_out_rewind (struct ui_out *uiout); extern void mi_out_buffered (struct ui_out *uiout, char *string);