Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA] Change exceptions.h functions to use gdb::function_view
@ 2017-09-27 16:53 Tom Tromey
  2017-09-27 19:43 ` Pedro Alves
  0 siblings, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2017-09-27 16:53 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes some functions in exceptions.h to use gdb::function_view,
then fixes up the fallout.  This lead to some surprising places, like
a function pointer in target_so_ops.

While writing this I found that catch_exception_ftype was unused, so I
removed this.

Note that I did not compile the windows-nat.c change, so I don't know
if it works.

Regression tested by the buildbot.

gdb/ChangeLog
2017-09-27  Tom Tromey  <tom@tromey.com>

	* windows-nat.c (get_windows_debug_event): Update.
	(handle_load_dll): Remove argument.
	* solib-target.c (solib_target_open_symbol_file_object): Change
	type.
	* solib-svr4.c (open_symbol_file_object): Change type.
	* solib-frv.c (open_symbol_file_object): Change type.
	* solib-dsbt.c (open_symbol_file_object): Change type.
	* solib-darwin.c (open_symbol_file_object): Change type.
	* solib-aix.c (solib_aix_open_symbol_file_object): Change type.
	* thread.c (do_captured_list_thread_ids): Remove argument.
	(enum gdb_rc): Update.
	(do_captured_thread_select): Change type.
	(enum gdb_rc): Update.
	* symmisc.c (struct print_symbol_args): Remove.
	(print_symbol): Change type.
	(dump_symtab_1): Update.
	* solib.c (update_solib_list): Update.
	* solist.h (struct target_so_ops) <open_symbol_file_object>:
	Change type.
	* record-full.c (struct record_full_message_args): Remove.
	(record_full_message_wrapper): Remove.
	(record_full_message_wrapper_safe): Update.
	* objc-lang.c (find_objc_msgcall_submethod_helper): Remove.
	(find_objc_msgcall_submethod): Update.
	(struct objc_submethod_helper_data): Remove.
	* main.c (captured_command_loop): Remove parameter.
	(captured_main): Update.
	* infrun.c (hook_stop_stub): Remove.
	(normal_stop): Update.
	(restore_selected_frame): Change type.
	(restore_infcall_control_state): Update.
	* breakpoint.c (bpstat_check_watchpoint)
	(bpstat_check_breakpoint_conditions, gdb_breakpoint_query)
	(struct captured_breakpoint_query_args): Remove.
	(do_captured_breakpoint_query): Change type.
	(breakpoint_re_set): Update.
	* symfile-mem.c (add_vsyscall_page): Update.
	* remote-fileio.c (remote_fileio_request): Update.
	* exceptions.c (catch_exceptions): Use gdb::function_view.
	(catch_exceptions_with_msg, catch_errors): Likewise.
	* exceptions.h (catch_errors, catch_exceptions_with_msg)
	(catch_errors): Update.
	(catch_exceptions_ftype): Remove args.
	(catch_exception_ftype): Remove.
---
 gdb/ChangeLog       | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 gdb/breakpoint.c    | 41 +++++++++++++++++++++++------------------
 gdb/exceptions.c    | 14 ++++++--------
 gdb/exceptions.h    | 19 +++++++++----------
 gdb/infrun.c        | 25 ++++++++++---------------
 gdb/main.c          |  4 ++--
 gdb/objc-lang.c     | 33 +++++++--------------------------
 gdb/record-full.c   | 27 +++++----------------------
 gdb/remote-fileio.c |  5 ++++-
 gdb/solib-aix.c     |  2 +-
 gdb/solib-darwin.c  |  2 +-
 gdb/solib-dsbt.c    | 10 ++--------
 gdb/solib-frv.c     |  2 +-
 gdb/solib-svr4.c    | 10 ++--------
 gdb/solib-target.c  |  2 +-
 gdb/solib.c         |  5 ++++-
 gdb/solist.h        |  7 ++-----
 gdb/symfile-mem.c   |  9 +++++++--
 gdb/symmisc.c       | 29 ++++++++++++-----------------
 gdb/thread.c        | 16 ++++++++++------
 gdb/windows-nat.c   |  6 +++---
 21 files changed, 159 insertions(+), 156 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 59cb354..d4ae985 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5189,8 +5189,11 @@ bpstat_check_watchpoint (bpstat bs)
 	    = xstrprintf ("Error evaluating expression for watchpoint %d\n",
 			  b->number);
 	  struct cleanup *cleanups = make_cleanup (xfree, message);
-	  int e = catch_errors (watchpoint_check, bs, message,
-				RETURN_MASK_ALL);
+	  int e = catch_errors ([&] ()
+				  {
+				    return watchpoint_check (bs);
+				  },
+	                        message, RETURN_MASK_ALL);
 	  do_cleanups (cleanups);
 	  switch (e)
 	    {
@@ -5421,7 +5424,10 @@ bpstat_check_breakpoint_conditions (bpstat bs, ptid_t ptid)
 	}
       if (within_current_scope)
 	value_is_zero
-	  = catch_errors (breakpoint_cond_eval, cond,
+	  = catch_errors ([&] ()
+			    {
+			      return breakpoint_cond_eval (cond);
+			    },
 			  "Error in testing breakpoint condition:\n",
 			  RETURN_MASK_ALL);
       else
@@ -6597,22 +6603,15 @@ breakpoint_address_bits (struct breakpoint *b)
   return print_address_bits;
 }
 
-struct captured_breakpoint_query_args
-  {
-    int bnum;
-  };
-
 static int
-do_captured_breakpoint_query (struct ui_out *uiout, void *data)
+do_captured_breakpoint_query (int bnum)
 {
-  struct captured_breakpoint_query_args *args
-    = (struct captured_breakpoint_query_args *) data;
   struct breakpoint *b;
   struct bp_location *dummy_loc = NULL;
 
   ALL_BREAKPOINTS (b)
     {
-      if (args->bnum == b->number)
+      if (bnum == b->number)
 	{
 	  print_one_breakpoint (b, &dummy_loc, 0);
 	  return GDB_RC_OK;
@@ -6625,13 +6624,15 @@ enum gdb_rc
 gdb_breakpoint_query (struct ui_out *uiout, int bnum, 
 		      char **error_message)
 {
-  struct captured_breakpoint_query_args args;
-
-  args.bnum = bnum;
   /* For the moment we don't trust print_one_breakpoint() to not throw
      an error.  */
-  if (catch_exceptions_with_msg (uiout, do_captured_breakpoint_query, &args,
-				 error_message, RETURN_MASK_ALL) < 0)
+  if (catch_exceptions_with_msg
+      (uiout,
+       [&] (struct ui_out *)
+         {
+	   return do_captured_breakpoint_query (bnum);
+	 },
+       error_message, RETURN_MASK_ALL) < 0)
     return GDB_RC_FAIL;
   else
     return GDB_RC_OK;
@@ -14192,7 +14193,11 @@ breakpoint_re_set (void)
 	char *message = xstrprintf ("Error in re-setting breakpoint %d: ",
 				    b->number);
 	struct cleanup *cleanups = make_cleanup (xfree, message);
-	catch_errors (breakpoint_re_set_one, b, message, RETURN_MASK_ALL);
+	catch_errors ([&] ()
+		        {
+			  return breakpoint_re_set_one (b);
+			},
+	              message, RETURN_MASK_ALL);
 	do_cleanups (cleanups);
       }
     set_language (save_language);
diff --git a/gdb/exceptions.c b/gdb/exceptions.c
index dd11b6e..bbd97cc 100644
--- a/gdb/exceptions.c
+++ b/gdb/exceptions.c
@@ -155,17 +155,15 @@ exception_fprintf (struct ui_file *file, struct gdb_exception e,
 
 int
 catch_exceptions (struct ui_out *uiout,
-		  catch_exceptions_ftype *func,
-		  void *func_args,
+		  gdb::function_view<catch_exceptions_ftype> func,
 		  return_mask mask)
 {
-  return catch_exceptions_with_msg (uiout, func, func_args, NULL, mask);
+  return catch_exceptions_with_msg (uiout, func, NULL, mask);
 }
 
 int
 catch_exceptions_with_msg (struct ui_out *func_uiout,
-		  	   catch_exceptions_ftype *func,
-		  	   void *func_args,
+			   gdb::function_view<catch_exceptions_ftype> func,
 			   char **gdberrmsg,
 		  	   return_mask mask)
 {
@@ -179,7 +177,7 @@ catch_exceptions_with_msg (struct ui_out *func_uiout,
 
   TRY
     {
-      val = (*func) (current_uiout, func_args);
+      val = func (current_uiout);
     }
   CATCH (ex, RETURN_MASK_ALL)
     {
@@ -220,7 +218,7 @@ catch_exceptions_with_msg (struct ui_out *func_uiout,
 /* This function is superseded by catch_exceptions().  */
 
 int
-catch_errors (catch_errors_ftype *func, void *func_args,
+catch_errors (gdb::function_view<catch_errors_ftype> func,
 	      const char *errstring, return_mask mask)
 {
   struct gdb_exception exception = exception_none;
@@ -232,7 +230,7 @@ catch_errors (catch_errors_ftype *func, void *func_args,
 
   TRY
     {
-      val = func (func_args);
+      val = func ();
     }
   CATCH (ex, RETURN_MASK_ALL)
     {
diff --git a/gdb/exceptions.h b/gdb/exceptions.h
index b2cdee3..c65974d 100644
--- a/gdb/exceptions.h
+++ b/gdb/exceptions.h
@@ -65,16 +65,15 @@ extern void exception_fprintf (struct ui_file *file, struct gdb_exception e,
    This function uses SETJMP() and LONGJUMP().  */
 
 struct ui_out;
-typedef int (catch_exceptions_ftype) (struct ui_out *ui_out, void *args);
+typedef int (catch_exceptions_ftype) (struct ui_out *ui_out);
 extern int catch_exceptions (struct ui_out *uiout,
-			     catch_exceptions_ftype *func, void *func_args,
+			     gdb::function_view<catch_exceptions_ftype> func,
 			     return_mask mask);
-typedef void (catch_exception_ftype) (struct ui_out *ui_out, void *args);
-extern int catch_exceptions_with_msg (struct ui_out *uiout,
-			     	      catch_exceptions_ftype *func, 
-			     	      void *func_args,
-			     	      char **gdberrmsg,
-				      return_mask mask);
+extern int catch_exceptions_with_msg
+    (struct ui_out *uiout,
+     gdb::function_view<catch_exceptions_ftype> func, 
+     char **gdberrmsg,
+     return_mask mask);
 
 /* If CATCH_ERRORS_FTYPE throws an error, catch_errors() returns zero
    otherwize the result from CATCH_ERRORS_FTYPE is returned.  It is
@@ -85,8 +84,8 @@ extern int catch_exceptions_with_msg (struct ui_out *uiout,
 
    This function is superseded by catch_exceptions().  */
 
-typedef int (catch_errors_ftype) (void *);
-extern int catch_errors (catch_errors_ftype *, void *,
+typedef int (catch_errors_ftype) ();
+extern int catch_errors (gdb::function_view<catch_errors_ftype>,
 			 const char *, return_mask);
 
 /* Compare two exception objects for print equality.  */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index f44ca91..149a358 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -80,10 +80,6 @@ static void sig_print_header (void);
 
 static void resume_cleanups (void *);
 
-static int hook_stop_stub (void *);
-
-static int restore_selected_frame (void *);
-
 static int follow_fork (void);
 
 static int follow_fork_inferior (int follow_child, int detach_fork);
@@ -8312,7 +8308,11 @@ normal_stop (void)
       struct cleanup *old_chain
 	= make_cleanup (release_stop_context_cleanup, saved_context);
 
-      catch_errors (hook_stop_stub, stop_command,
+      catch_errors ([&] ()
+		      {
+			execute_cmd_pre_hook (stop_command);
+			return 0;
+		      },
 		    "Error while running hook_stop:\n", RETURN_MASK_ALL);
 
       /* If the stop hook resumes the target, then there's no point in
@@ -8354,13 +8354,6 @@ normal_stop (void)
 
   return 0;
 }
-
-static int
-hook_stop_stub (void *cmd)
-{
-  execute_cmd_pre_hook ((struct cmd_list_element *) cmd);
-  return (0);
-}
 \f
 int
 signal_stop_state (int signo)
@@ -8982,9 +8975,8 @@ save_infcall_control_state (void)
 }
 
 static int
-restore_selected_frame (void *args)
+restore_selected_frame (struct frame_id *fid)
 {
-  struct frame_id *fid = (struct frame_id *) args;
   struct frame_info *frame;
 
   frame = frame_find_by_id (*fid);
@@ -9033,7 +9025,10 @@ restore_infcall_control_state (struct infcall_control_state *inf_status)
          walking the stack might encounter a garbage pointer and
          error() trying to dereference it.  */
       if (catch_errors
-	  (restore_selected_frame, &inf_status->selected_frame_id,
+	  ([&] ()
+	     {
+	       return restore_selected_frame (&inf_status->selected_frame_id);
+	     },
 	   "Unable to restore previously selected frame:\n",
 	   RETURN_MASK_ERROR) == 0)
 	/* Error in restoring the selected frame.  Select the innermost
diff --git a/gdb/main.c b/gdb/main.c
index 66ba75b..8408bfc 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -309,7 +309,7 @@ setup_alternate_signal_stack (void)
    non-zero return status.  */
 
 static int
-captured_command_loop (void *data)
+captured_command_loop ()
 {
   struct ui *ui = current_ui;
 
@@ -1147,7 +1147,7 @@ captured_main (void *data)
      change - SET_TOP_LEVEL() - has been eliminated.  */
   while (1)
     {
-      catch_errors (captured_command_loop, 0, "", RETURN_MASK_ALL);
+      catch_errors (captured_command_loop, "", RETURN_MASK_ALL);
     }
   /* No exit -- exit is through quit_command.  */
 }
diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
index dc9c934..2b84904 100644
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -1310,37 +1310,18 @@ find_objc_msgsend (void)
  * dependent modules.
  */
 
-struct objc_submethod_helper_data {
-  int (*f) (CORE_ADDR, CORE_ADDR *);
-  CORE_ADDR pc;
-  CORE_ADDR *new_pc;
-};
-
-static int 
-find_objc_msgcall_submethod_helper (void * arg)
-{
-  struct objc_submethod_helper_data *s = 
-    (struct objc_submethod_helper_data *) arg;
-
-  if (s->f (s->pc, s->new_pc) == 0) 
-    return 1;
-  else 
-    return 0;
-}
-
 static int 
 find_objc_msgcall_submethod (int (*f) (CORE_ADDR, CORE_ADDR *),
 			     CORE_ADDR pc, 
 			     CORE_ADDR *new_pc)
 {
-  struct objc_submethod_helper_data s;
-
-  s.f = f;
-  s.pc = pc;
-  s.new_pc = new_pc;
-
-  if (catch_errors (find_objc_msgcall_submethod_helper,
-		    (void *) &s,
+  if (catch_errors ([&] ()
+		      {
+			if (f (pc, new_pc) == 0)
+			  return 1;
+			else
+			  return 0;
+		      },
 		    "Unable to determine target of "
 		    "Objective-C method call (ignoring):\n",
 		    RETURN_MASK_ALL) == 0) 
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 5073be3..2d9085a 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -637,32 +637,15 @@ record_full_message (struct regcache *regcache, enum gdb_signal signal)
   return 1;
 }
 
-struct record_full_message_args {
-  struct regcache *regcache;
-  enum gdb_signal signal;
-};
-
-static int
-record_full_message_wrapper (void *args)
-{
-  struct record_full_message_args *record_full_args
-    = (struct record_full_message_args *) args;
-
-  return record_full_message (record_full_args->regcache,
-			      record_full_args->signal);
-}
-
 static int
 record_full_message_wrapper_safe (struct regcache *regcache,
 				  enum gdb_signal signal)
 {
-  struct record_full_message_args args;
-
-  args.regcache = regcache;
-  args.signal = signal;
-
-  return catch_errors (record_full_message_wrapper, &args, "",
-		       RETURN_MASK_ALL);
+  return catch_errors ([&] ()
+		         {
+			   return record_full_message (regcache, signal);
+			 },
+		       "", RETURN_MASK_ALL);
 }
 
 /* Set to 1 if record_full_store_registers and record_full_xfer_partial
diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c
index c305171..f3e33ec 100644
--- a/gdb/remote-fileio.c
+++ b/gdb/remote-fileio.c
@@ -1189,7 +1189,10 @@ remote_fileio_request (char *buf, int ctrlc_pending_p)
   else
     {
       ex = catch_exceptions (current_uiout,
-			     do_remote_fileio_request, (void *)buf,
+			     [&] (ui_out *uiout)
+			       {
+				 return do_remote_fileio_request (uiout, buf);
+			       },
 			     RETURN_MASK_ALL);
       switch (ex)
 	{
diff --git a/gdb/solib-aix.c b/gdb/solib-aix.c
index 633c9e6..9233e78 100644
--- a/gdb/solib-aix.c
+++ b/gdb/solib-aix.c
@@ -589,7 +589,7 @@ solib_aix_current_sos (void)
 /* Implement the "open_symbol_file_object" target_so_ops method.  */
 
 static int
-solib_aix_open_symbol_file_object (void *from_ttyp)
+solib_aix_open_symbol_file_object (int from_tty)
 {
   return 0;
 }
diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
index 04bbf86..f63f924 100644
--- a/gdb/solib-darwin.c
+++ b/gdb/solib-darwin.c
@@ -222,7 +222,7 @@ find_program_interpreter (void)
     Note that darwin-nat.c implements pid_to_exec_file.  */
 
 static int
-open_symbol_file_object (void *from_ttyp)
+open_symbol_file_object (int from_tty)
 {
   return 0;
 }
diff --git a/gdb/solib-dsbt.c b/gdb/solib-dsbt.c
index 86b9e30..0b4055c 100644
--- a/gdb/solib-dsbt.c
+++ b/gdb/solib-dsbt.c
@@ -507,16 +507,10 @@ scan_dyntag (int dyntag, bfd *abfd, CORE_ADDR *ptr)
   return 0;
 }
 
-/* If no open symbol file, attempt to locate and open the main symbol
-   file.
-
-   If FROM_TTYP dereferences to a non-zero integer, allow messages to
-   be printed.  This parameter is a pointer rather than an int because
-   open_symbol_file_object is called via catch_errors and
-   catch_errors requires a pointer argument. */
+/* See solist.h. */
 
 static int
-open_symbol_file_object (void *from_ttyp)
+open_symbol_file_object (int from_tty)
 {
   /* Unimplemented.  */
   return 0;
diff --git a/gdb/solib-frv.c b/gdb/solib-frv.c
index 6d74652..fd748f4 100644
--- a/gdb/solib-frv.c
+++ b/gdb/solib-frv.c
@@ -246,7 +246,7 @@ static int enable_break2 (void);
 /* Implement the "open_symbol_file_object" target_so_ops method.  */
 
 static int
-open_symbol_file_object (void *from_ttyp)
+open_symbol_file_object (int from_tty)
 {
   /* Unimplemented.  */
   return 0;
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 405de37..d334791 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -984,20 +984,14 @@ svr4_keep_data_in_core (CORE_ADDR vaddr, unsigned long size)
   return (name_lm >= vaddr && name_lm < vaddr + size);
 }
 
-/* Implement the "open_symbol_file_object" target_so_ops method.
-
-   If no open symbol file, attempt to locate and open the main symbol
-   file.  On SVR4 systems, this is the first link map entry.  If its
-   name is here, we can open it.  Useful when attaching to a process
-   without first loading its symbol file.  */
+/* See solist.h.  */
 
 static int
-open_symbol_file_object (void *from_ttyp)
+open_symbol_file_object (int from_tty)
 {
   CORE_ADDR lm, l_name;
   char *filename;
   int errcode;
-  int from_tty = *(int *)from_ttyp;
   struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
   struct type *ptr_type = builtin_type (target_gdbarch ())->builtin_data_ptr;
   int l_name_size = TYPE_LENGTH (ptr_type);
diff --git a/gdb/solib-target.c b/gdb/solib-target.c
index dcd0f85..cf569ec 100644
--- a/gdb/solib-target.c
+++ b/gdb/solib-target.c
@@ -452,7 +452,7 @@ Could not relocate shared library \"%s\": bad offsets"), so->so_name);
 }
 
 static int
-solib_target_open_symbol_file_object (void *from_ttyp)
+solib_target_open_symbol_file_object (int from_tty)
 {
   /* We can't locate the main symbol file based on the target's
      knowledge; the user has to specify it.  */
diff --git a/gdb/solib.c b/gdb/solib.c
index 4f7fd90..0349c8f 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -760,7 +760,10 @@ update_solib_list (int from_tty)
 	 have not opened a symbol file, we may be able to get its
 	 symbols now!  */
       if (inf->attach_flag && symfile_objfile == NULL)
-	catch_errors (ops->open_symbol_file_object, &from_tty,
+	catch_errors ([&] ()
+		        {
+			  return ops->open_symbol_file_object (from_tty);
+			},
 		      "Error reading attached process's symbol file.\n",
 		      RETURN_MASK_ALL);
     }
diff --git a/gdb/solist.h b/gdb/solist.h
index 5eb2d39..992986e 100644
--- a/gdb/solist.h
+++ b/gdb/solist.h
@@ -121,11 +121,8 @@ struct target_so_ops
     struct so_list *(*current_sos) (void);
 
     /* Find, open, and read the symbols for the main executable.  If
-       FROM_TTYP dereferences to a non-zero integer, allow messages to
-       be printed.  This parameter is a pointer rather than an int
-       because open_symbol_file_object is called via catch_errors and
-       catch_errors requires a pointer argument.  */
-    int (*open_symbol_file_object) (void *from_ttyp);
+       FROM_TTY is non-zero, allow messages to be printed.  */
+    int (*open_symbol_file_object) (int from_ttyp);
 
     /* Determine if PC lies in the dynamic symbol resolution code of
        the run time loader.  */
diff --git a/gdb/symfile-mem.c b/gdb/symfile-mem.c
index 6f34cab..5cefbc9 100644
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -231,8 +231,13 @@ add_vsyscall_page (struct target_ops *target, int from_tty)
 	 vsyscall DSO was not triggered by the user, even if the user
 	 typed "run" at the TTY.  */
       args.from_tty = 0;
-      catch_exceptions (current_uiout, symbol_file_add_from_memory_wrapper,
-			&args, RETURN_MASK_ALL);
+      catch_exceptions (current_uiout,
+			[&] (ui_out *uiout)
+			  {
+			    return symbol_file_add_from_memory_wrapper (uiout,
+									&args);
+			  },
+			RETURN_MASK_ALL);
     }
 }
 
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index 86422e7..6236bad 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -62,7 +62,8 @@ struct print_symbol_args
     struct ui_file *outfile;
   };
 
-static int print_symbol (void *);
+static int print_symbol (struct gdbarch *gdbarch, struct symbol *symbol,
+			 int depth, struct ui_file *outfile);
 \f
 
 void
@@ -357,13 +358,12 @@ dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile)
 	     block, not any blocks from included symtabs.  */
 	  ALL_DICT_SYMBOLS (BLOCK_DICT (b), iter, sym)
 	    {
-	      struct print_symbol_args s;
-
-	      s.gdbarch = gdbarch;
-	      s.symbol = sym;
-	      s.depth = depth + 1;
-	      s.outfile = outfile;
-	      catch_errors (print_symbol, &s, "Error printing symbol:\n",
+	      catch_errors ([&] ()
+			      {
+				return print_symbol (gdbarch, sym, depth + 1,
+						     outfile);
+			      },
+		            "Error printing symbol:\n",
 			    RETURN_MASK_ERROR);
 	    }
 	}
@@ -515,18 +515,13 @@ maintenance_print_symbols (const char *args, int from_tty)
     }
 }
 
-/* Print symbol ARGS->SYMBOL on ARGS->OUTFILE.  ARGS->DEPTH says how
-   far to indent.  ARGS is really a struct print_symbol_args *, but is
-   declared as char * to get it past catch_errors.  Returns 0 for error,
-   1 for success.  */
+/* Print symbol SYMBOL on OUTFILE.  DEPTH says how far to indent.
+   Returns 0 for error, 1 for success.  */
 
 static int
-print_symbol (void *args)
+print_symbol (struct gdbarch *gdbarch, struct symbol *symbol,
+	      int depth, struct ui_file *outfile)
 {
-  struct gdbarch *gdbarch = ((struct print_symbol_args *) args)->gdbarch;
-  struct symbol *symbol = ((struct print_symbol_args *) args)->symbol;
-  int depth = ((struct print_symbol_args *) args)->depth;
-  struct ui_file *outfile = ((struct print_symbol_args *) args)->outfile;
   struct obj_section *section;
 
   if (SYMBOL_OBJFILE_OWNED (symbol))
diff --git a/gdb/thread.c b/gdb/thread.c
index 91c077a..7deebf2 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -723,7 +723,7 @@ any_live_thread_of_process (int pid)
 /* Print a list of thread ids currently known, and the total number of
    threads.  To be used from within catch_errors.  */
 static int
-do_captured_list_thread_ids (struct ui_out *uiout, void *arg)
+do_captured_list_thread_ids (struct ui_out *uiout)
 {
   struct thread_info *tp;
   int num = 0;
@@ -758,7 +758,7 @@ do_captured_list_thread_ids (struct ui_out *uiout, void *arg)
 enum gdb_rc
 gdb_list_thread_ids (struct ui_out *uiout, char **error_message)
 {
-  if (catch_exceptions_with_msg (uiout, do_captured_list_thread_ids, NULL,
+  if (catch_exceptions_with_msg (uiout, do_captured_list_thread_ids,
 				 error_message, RETURN_MASK_ALL) < 0)
     return GDB_RC_FAIL;
   return GDB_RC_OK;
@@ -1992,9 +1992,8 @@ show_print_thread_events (struct ui_file *file, int from_tty,
 }
 
 static int
-do_captured_thread_select (struct ui_out *uiout, void *tidstr_v)
+do_captured_thread_select (struct ui_out *uiout, const char *tidstr)
 {
-  const char *tidstr = (const char *) tidstr_v;
   struct thread_info *tp;
 
   if (uiout->is_mi_like_p ())
@@ -2070,8 +2069,13 @@ print_selected_thread_frame (struct ui_out *uiout,
 enum gdb_rc
 gdb_thread_select (struct ui_out *uiout, char *tidstr, char **error_message)
 {
-  if (catch_exceptions_with_msg (uiout, do_captured_thread_select, tidstr,
-				 error_message, RETURN_MASK_ALL) < 0)
+  if (catch_exceptions_with_msg
+      (uiout,
+       [&] (struct ui_out *inner_uiout)
+         {
+	   return do_captured_thread_select (inner_uiout, tidstr);
+	 },
+       error_message, RETURN_MASK_ALL) < 0)
     return GDB_RC_FAIL;
   return GDB_RC_OK;
 }
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 3e18944..8fb6595 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -755,7 +755,7 @@ get_image_name (HANDLE h, void *address, int unicode)
    on how we handle DLL loading during that phase).  */
 
 static int
-handle_load_dll (void *dummy)
+handle_load_dll ()
 {
   LOAD_DLL_DEBUG_INFO *event = &current_event.u.LoadDll;
   char *dll_name;
@@ -1540,7 +1540,7 @@ get_windows_debug_event (struct target_ops *ops,
       CloseHandle (current_event.u.LoadDll.hFile);
       if (saw_create != 1 || ! windows_initialization_done)
 	break;
-      catch_errors (handle_load_dll, NULL, (char *) "", RETURN_MASK_ALL);
+      catch_errors (handle_load_dll, (char *) "", RETURN_MASK_ALL);
       ourstatus->kind = TARGET_WAITKIND_LOADED;
       ourstatus->value.integer = 0;
       thread_id = main_thread_id;
@@ -1553,7 +1553,7 @@ get_windows_debug_event (struct target_ops *ops,
 		     "UNLOAD_DLL_DEBUG_EVENT"));
       if (saw_create != 1 || ! windows_initialization_done)
 	break;
-      catch_errors (handle_unload_dll, NULL, (char *) "", RETURN_MASK_ALL);
+      catch_errors (handle_unload_dll, (char *) "", RETURN_MASK_ALL);
       ourstatus->kind = TARGET_WAITKIND_LOADED;
       ourstatus->value.integer = 0;
       thread_id = main_thread_id;
-- 
2.9.5


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFA] Change exceptions.h functions to use gdb::function_view
  2017-09-27 16:53 [RFA] Change exceptions.h functions to use gdb::function_view Tom Tromey
@ 2017-09-27 19:43 ` Pedro Alves
  2017-09-27 20:26   ` Tom Tromey
  0 siblings, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2017-09-27 19:43 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 09/27/2017 05:53 PM, Tom Tromey wrote:
> This changes some functions in exceptions.h to use gdb::function_view,
> then fixes up the fallout.  This lead to some surprising places, like
> a function pointer in target_so_ops.
> 
> While writing this I found that catch_exception_ftype was unused, so I
> removed this.
> 
> Note that I did not compile the windows-nat.c change, so I don't know
> if it works.
> 
> Regression tested by the buildbot.
> 

I'm borderline about this.  I have to say that I question the value of
catch_exceptions&co, over just using TRY/CATCH + a scoped_restore(current_uiout)
in the try scope + printing the exception.  A TRY/CATCH is likely to be
easier to understand and debug, I think.  I mean, take the print_symbol
case [it was just the random one that I picked], and compare:

             TRY
               {
                 print_symbol (gdbarch, sym, depth + 1, outfile);
               }
             CATCH (ex, RETURN_MASK_ERROR)
               {
                 exception_fprintf (gdb_stderr, ex,
                                    "Error printing symbol:\n");
               }

vs
    
	      catch_errors ([&] ()
			      {
				return print_symbol (gdbarch, sym, depth + 1,
						     outfile);
			      },
		            "Error printing symbol:\n",
 			    RETURN_MASK_ERROR);


It seems like that case doesn't even need a scoped_restore for current_uiout.
And with TRY/CATCH, print_symbol can be simplified further to return void.

Did you consider this?

> -  if (catch_exceptions_with_msg (uiout, do_captured_breakpoint_query, &args,
> -				 error_message, RETURN_MASK_ALL) < 0)
> +  if (catch_exceptions_with_msg
> +      (uiout,
> +       [&] (struct ui_out *)
> +         {
> +	   return do_captured_breakpoint_query (bnum);
> +	 },

We don't really need the ui_out * parameter in
catch_exceptions_with_msg's callback anymore, since
you can always access it via lambda capture.  Would you
consider removing it?

>  
>  static int
> -do_captured_thread_select (struct ui_out *uiout, void *tidstr_v)
> +do_captured_thread_select (struct ui_out *uiout, const char *tidstr)
>  {


> +         {
> +	   return do_captured_thread_select (inner_uiout, tidstr);
> +	 },

Note the patch has several cases of tabs vs spaces like above.


> @@ -1540,7 +1540,7 @@ get_windows_debug_event (struct target_ops *ops,
>        CloseHandle (current_event.u.LoadDll.hFile);
>        if (saw_create != 1 || ! windows_initialization_done)
>  	break;
> -      catch_errors (handle_load_dll, NULL, (char *) "", RETURN_MASK_ALL);
> +      catch_errors (handle_load_dll, (char *) "", RETURN_MASK_ALL);
>        ourstatus->kind = TARGET_WAITKIND_LOADED;
>        ourstatus->value.integer = 0;
>        thread_id = main_thread_id;
> @@ -1553,7 +1553,7 @@ get_windows_debug_event (struct target_ops *ops,
>  		     "UNLOAD_DLL_DEBUG_EVENT"));
>        if (saw_create != 1 || ! windows_initialization_done)
>  	break;
> -      catch_errors (handle_unload_dll, NULL, (char *) "", RETURN_MASK_ALL);
> +      catch_errors (handle_unload_dll, (char *) "", RETURN_MASK_ALL);
>        ourstatus->kind = TARGET_WAITKIND_LOADED;
>        ourstatus->value.integer = 0;
>        thread_id = main_thread_id;

I don't think we need the casts nowadays.  catch_errors takes a const string (since the -Wwrite-strings patch).

Thanks,
Pedro Alves


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFA] Change exceptions.h functions to use gdb::function_view
  2017-09-27 19:43 ` Pedro Alves
@ 2017-09-27 20:26   ` Tom Tromey
  2017-09-27 22:37     ` Pedro Alves
  0 siblings, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2017-09-27 20:26 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> I'm borderline about this.  I have to say that I question the value of
Pedro> catch_exceptions&co, over just using TRY/CATCH + a scoped_restore(current_uiout)
Pedro> in the try scope + printing the exception.  A TRY/CATCH is likely to be
Pedro> easier to understand and debug, I think.
[...]
Pedro> Did you consider this?

Nope.  This was actually just a preparatory patch for another const change.

I can look into the change though.  I agree it would be better.

>> +         {
>> +	   return do_captured_thread_select (inner_uiout, tidstr);
>> +	 },

Pedro> Note the patch has several cases of tabs vs spaces like above.

Sorry about that.
Was there ever a decision about not using tabs?

In this case I think the problem is that Emacs isn't indenting lambdas
well.

Pedro> I don't think we need the casts nowadays.  catch_errors takes a const
Pedro> string (since the -Wwrite-strings patch).

Ok.

Tom


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFA] Change exceptions.h functions to use gdb::function_view
  2017-09-27 20:26   ` Tom Tromey
@ 2017-09-27 22:37     ` Pedro Alves
  2017-09-28  0:02       ` [users/palves/catch_exceptions] [PATCH] zap catch_exceptions (Re: [RFA] Change exceptions.h functions to use gdb::function_view) Pedro Alves
  0 siblings, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2017-09-27 22:37 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 09/27/2017 09:25 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> I'm borderline about this.  I have to say that I question the value of
> Pedro> catch_exceptions&co, over just using TRY/CATCH + a scoped_restore(current_uiout)
> Pedro> in the try scope + printing the exception.  A TRY/CATCH is likely to be
> Pedro> easier to understand and debug, I think.
> [...]
> Pedro> Did you consider this?
> 
> Nope.  This was actually just a preparatory patch for another const change.
> 
> I can look into the change though.  I agree it would be better.

I took a stab at this.  I converted catch_errors to see what it
looks like.  See below a(n untested) patch on top of yours.  

This exposed further cleanup opportunities around
no longer having to follow catch_errors callback type, and
also removes a few cleanups.

I didn't do anything to save/restore current_uiout because it
feels to me like that should be the responsibility of the code
that changes current_uiout in the first place.

For the the other catch_foo functions, we may need to write
scoped restores.

> 
>>> +         {
>>> +	   return do_captured_thread_select (inner_uiout, tidstr);
>>> +	 },
> 
> Pedro> Note the patch has several cases of tabs vs spaces like above.
> 
> Sorry about that.
> Was there ever a decision about not using tabs?
> 

Nope.

I push this to the users/palves/catch_exceptions branch too.

From bea0a5aeece991b2f4f20a746ce655283607d9a6 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 27 Sep 2017 22:43:26 +0100
Subject: [PATCH] zap catch_errors

---
 gdb/breakpoint.c  | 155 +++++++++++++++++++++++++-----------------------------
 gdb/exceptions.c  |  39 --------------
 gdb/exceptions.h  |  32 +++--------
 gdb/infrun.c      |  41 +++++++++------
 gdb/main.c        |  10 +++-
 gdb/objc-lang.c   |  26 ++++-----
 gdb/record-full.c |  23 ++++----
 gdb/solib.c       |  19 ++++---
 gdb/symmisc.c     |  37 ++++++-------
 9 files changed, 167 insertions(+), 215 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index d4ae985..02e028c 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -103,8 +103,6 @@ static void map_breakpoint_numbers (const char *,
 
 static void ignore_command (char *, int);
 
-static int breakpoint_re_set_one (void *);
-
 static void breakpoint_re_set_default (struct breakpoint *);
 
 static void
@@ -178,8 +176,6 @@ static void info_breakpoints_command (char *, int);
 
 static void info_watchpoints_command (char *, int);
 
-static int breakpoint_cond_eval (void *);
-
 static void cleanup_executing_breakpoints (void *);
 
 static void commands_command (char *, int);
@@ -191,8 +187,6 @@ static int remove_breakpoint_1 (struct bp_location *, enum remove_bp_reason);
 
 static enum print_stop_action print_bp_stop_message (bpstat bs);
 
-static int watchpoint_check (void *);
-
 static int hw_breakpoint_used_count (void);
 
 static int hw_watchpoint_use_count (struct breakpoint *);
@@ -4842,21 +4836,16 @@ bpstat_print (bpstat bs, int kind)
   return PRINT_UNKNOWN;
 }
 
-/* Evaluate the expression EXP and return 1 if value is zero.
-   This returns the inverse of the condition because it is called
-   from catch_errors which returns 0 if an exception happened, and if an
-   exception happens we want execution to stop.
-   The argument is a "struct expression *" that has been cast to a
-   "void *" to make it pass through catch_errors.  */
+/* Evaluate the boolean expression EXP and return the result.  */
 
-static int
-breakpoint_cond_eval (void *exp)
+static bool
+breakpoint_cond_eval (expression *exp)
 {
   struct value *mark = value_mark ();
-  int i = !value_true (evaluate_expression ((struct expression *) exp));
+  bool res = value_true (evaluate_expression (exp));
 
   value_free_to_mark (mark);
-  return i;
+  return res;
 }
 
 /* Allocate a new bpstat.  Link it to the FIFO list by BS_LINK_POINTER.  */
@@ -4966,30 +4955,28 @@ watchpoints_triggered (struct target_waitstatus *ws)
   return 1;
 }
 
-/* Possible return values for watchpoint_check (this can't be an enum
-   because of check_errors).  */
-/* The watchpoint has been deleted.  */
-#define WP_DELETED 1
-/* The value has changed.  */
-#define WP_VALUE_CHANGED 2
-/* The value has not changed.  */
-#define WP_VALUE_NOT_CHANGED 3
-/* Ignore this watchpoint, no matter if the value changed or not.  */
-#define WP_IGNORE 4
+/* Possible return values for watchpoint_check.  */
+enum wp_check_result
+  {
+    /* The watchpoint has been deleted.  */
+    WP_DELETED = 1,
+    /* The value has changed.  */
+    WP_VALUE_CHANGED = 2,
+    /* The value has not changed.  */
+    WP_VALUE_NOT_CHANGED = 3,
+    /* Ignore this watchpoint, no matter if the value changed or not.  */
+    WP_IGNORE = 4,
+  };
 
 #define BP_TEMPFLAG 1
 #define BP_HARDWAREFLAG 2
 
 /* Evaluate watchpoint condition expression and check if its value
-   changed.
-
-   P should be a pointer to struct bpstat, but is defined as a void *
-   in order for this function to be usable with catch_errors.  */
+   changed.  */
 
-static int
-watchpoint_check (void *p)
+static wp_check_result
+watchpoint_check (bpstat bs)
 {
-  bpstat bs = (bpstat) p;
   struct watchpoint *b;
   struct frame_info *fr;
   int within_current_scope;
@@ -5185,16 +5172,29 @@ bpstat_check_watchpoint (bpstat bs)
 
       if (must_check_value)
 	{
-	  char *message
-	    = xstrprintf ("Error evaluating expression for watchpoint %d\n",
-			  b->number);
-	  struct cleanup *cleanups = make_cleanup (xfree, message);
-	  int e = catch_errors ([&] ()
-				  {
-				    return watchpoint_check (bs);
-				  },
-	                        message, RETURN_MASK_ALL);
-	  do_cleanups (cleanups);
+	  wp_check_result e;
+
+	  TRY
+	    {
+	      e = watchpoint_check (bs);
+	    }
+	  CATCH (ex, RETURN_MASK_ALL)
+	    {
+	      exception_fprintf (gdb_stderr, ex,
+				 "Error evaluating expression "
+				 "for watchpoint %d\n",
+				 b->number);
+
+	      SWITCH_THRU_ALL_UIS ()
+		{
+		  printf_filtered (_("Watchpoint %d deleted.\n"),
+				   b->number);
+		}
+	      watchpoint_del_at_next_stop (b);
+	      e = WP_DELETED;
+	    }
+	  END_CATCH
+
 	  switch (e)
 	    {
 	    case WP_DELETED:
@@ -5290,18 +5290,6 @@ bpstat_check_watchpoint (bpstat bs)
 	      break;
 	    default:
 	      /* Can't happen.  */
-	    case 0:
-	      /* Error from catch_errors.  */
-	      {
-		SWITCH_THRU_ALL_UIS ()
-	          {
-		    printf_filtered (_("Watchpoint %d deleted.\n"),
-				     b->number);
-		  }
-		watchpoint_del_at_next_stop (b);
-		/* We've already printed what needs to be printed.  */
-		bs->print_it = print_it_done;
-	      }
 	      break;
 	    }
 	}
@@ -5327,7 +5315,8 @@ bpstat_check_breakpoint_conditions (bpstat bs, ptid_t ptid)
 {
   const struct bp_location *bl;
   struct breakpoint *b;
-  int value_is_zero = 0;
+  /* Assume stop.  */
+  bool condition_result = true;
   struct expression *cond;
 
   gdb_assert (bs->stop);
@@ -5423,26 +5412,30 @@ bpstat_check_breakpoint_conditions (bpstat bs, ptid_t ptid)
 	    within_current_scope = 0;
 	}
       if (within_current_scope)
-	value_is_zero
-	  = catch_errors ([&] ()
-			    {
-			      return breakpoint_cond_eval (cond);
-			    },
-			  "Error in testing breakpoint condition:\n",
-			  RETURN_MASK_ALL);
+	{
+	  TRY
+	    {
+	      condition_result = breakpoint_cond_eval (cond);
+	    }
+	  CATCH (ex, RETURN_MASK_ALL)
+	    {
+	      exception_fprintf (gdb_stderr, ex,
+				 "Error in testing breakpoint condition:\n");
+	    }
+	  END_CATCH
+	}
       else
 	{
 	  warning (_("Watchpoint condition cannot be tested "
 		     "in the current scope"));
 	  /* If we failed to set the right context for this
 	     watchpoint, unconditionally report it.  */
-	  value_is_zero = 0;
 	}
       /* FIXME-someday, should give breakpoint #.  */
       value_free_to_mark (mark);
     }
 
-  if (cond && value_is_zero)
+  if (cond && !condition_result)
     {
       bs->stop = 0;
     }
@@ -14149,21 +14142,16 @@ prepare_re_set_context (struct breakpoint *b)
   return make_cleanup (null_cleanup, NULL);
 }
 
-/* Reset a breakpoint given it's struct breakpoint * BINT.
-   The value we return ends up being the return value from catch_errors.
-   Unused in this case.  */
+/* Reset a breakpoint.  */
 
-static int
-breakpoint_re_set_one (void *bint)
+static void
+breakpoint_re_set_one (breakpoint *b)
 {
-  /* Get past catch_errs.  */
-  struct breakpoint *b = (struct breakpoint *) bint;
   struct cleanup *cleanups;
 
   cleanups = prepare_re_set_context (b);
   b->ops->re_set (b);
   do_cleanups (cleanups);
-  return 0;
 }
 
 /* Re-set breakpoint locations for the current program space.
@@ -14189,16 +14177,17 @@ breakpoint_re_set (void)
 
     ALL_BREAKPOINTS_SAFE (b, b_tmp)
       {
-	/* Format possible error msg.  */
-	char *message = xstrprintf ("Error in re-setting breakpoint %d: ",
-				    b->number);
-	struct cleanup *cleanups = make_cleanup (xfree, message);
-	catch_errors ([&] ()
-		        {
-			  return breakpoint_re_set_one (b);
-			},
-	              message, RETURN_MASK_ALL);
-	do_cleanups (cleanups);
+	TRY
+	  {
+	    breakpoint_re_set_one (b);
+	  }
+	CATCH (ex, RETURN_MASK_ALL)
+	  {
+	    exception_fprintf (gdb_stderr, ex,
+			       "Error in re-setting breakpoint %d: ",
+			       b->number);
+	  }
+	END_CATCH
       }
     set_language (save_language);
     input_radix = save_input_radix;
diff --git a/gdb/exceptions.c b/gdb/exceptions.c
index bbd97cc..97a56e6 100644
--- a/gdb/exceptions.c
+++ b/gdb/exceptions.c
@@ -215,45 +215,6 @@ catch_exceptions_with_msg (struct ui_out *func_uiout,
   return val;
 }
 
-/* This function is superseded by catch_exceptions().  */
-
-int
-catch_errors (gdb::function_view<catch_errors_ftype> func,
-	      const char *errstring, return_mask mask)
-{
-  struct gdb_exception exception = exception_none;
-  volatile int val = 0;
-  struct ui_out *saved_uiout;
-
-  /* Save the global ``struct ui_out'' builder.  */
-  saved_uiout = current_uiout;
-
-  TRY
-    {
-      val = func ();
-    }
-  CATCH (ex, RETURN_MASK_ALL)
-    {
-      exception = ex;
-    }
-  END_CATCH
-
-  /* Restore the global builder.  */
-  current_uiout = saved_uiout;
-
-  if (exception.reason < 0 && (mask & RETURN_MASK (exception.reason)) == 0)
-    {
-      /* The caller didn't request that the event be caught.
-	 Rethrow.  */
-      throw_exception (exception);
-    }
-
-  exception_fprintf (gdb_stderr, exception, "%s", errstring);
-  if (exception.reason != 0)
-    return 0;
-  return val;
-}
-
 /* See exceptions.h.  */
 
 int
diff --git a/gdb/exceptions.h b/gdb/exceptions.h
index c65974d..73aa870 100644
--- a/gdb/exceptions.h
+++ b/gdb/exceptions.h
@@ -48,21 +48,16 @@ extern void exception_fprintf (struct ui_file *file, struct gdb_exception e,
    copy of the gdb error message.  This is used when a silent error is 
    issued and the caller wants to manually issue the error message.
 
-   MASK specifies what to catch; it is normally set to
-   RETURN_MASK_ALL, if for no other reason than that the code which
-   calls catch_errors might not be set up to deal with a quit which
-   isn't caught.  But if the code can deal with it, it generally
-   should be RETURN_MASK_ERROR, unless for some reason it is more
-   useful to abort only the portion of the operation inside the
-   catch_errors.  Note that quit should return to the command line
+   MASK specifies what to catch; it is normally set to RETURN_MASK_ALL
+   if the code which calls catch_exceptions is not set up to deal with
+   a quit which isn't caught.  But if the code can deal with it, it
+   generally should be RETURN_MASK_ERROR, unless for some reason it is
+   more useful to abort only the portion of the operation inside the
+   catch_exceptions.  Note that quit should return to the command line
    fairly quickly, even if some further processing is being done.
 
    FIXME; cagney/2001-08-13: The need to override the global UIOUT
-   builder variable should just go away.
-
-   This function supersedes catch_errors().
-
-   This function uses SETJMP() and LONGJUMP().  */
+   builder variable should just go away.  */
 
 struct ui_out;
 typedef int (catch_exceptions_ftype) (struct ui_out *ui_out);
@@ -75,19 +70,6 @@ extern int catch_exceptions_with_msg
      char **gdberrmsg,
      return_mask mask);
 
-/* If CATCH_ERRORS_FTYPE throws an error, catch_errors() returns zero
-   otherwize the result from CATCH_ERRORS_FTYPE is returned.  It is
-   probably useful for CATCH_ERRORS_FTYPE to always return a non-zero
-   value.  It's unfortunate that, catch_errors() does not return an
-   indication of the exact exception that it caught - quit_flag might
-   help.
-
-   This function is superseded by catch_exceptions().  */
-
-typedef int (catch_errors_ftype) ();
-extern int catch_errors (gdb::function_view<catch_errors_ftype>,
-			 const char *, return_mask);
-
 /* Compare two exception objects for print equality.  */
 extern int exception_print_same (struct gdb_exception e1,
 				 struct gdb_exception e2);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 149a358..f8d5be1 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8308,12 +8308,16 @@ normal_stop (void)
       struct cleanup *old_chain
 	= make_cleanup (release_stop_context_cleanup, saved_context);
 
-      catch_errors ([&] ()
-		      {
-			execute_cmd_pre_hook (stop_command);
-			return 0;
-		      },
-		    "Error while running hook_stop:\n", RETURN_MASK_ALL);
+      TRY
+	{
+	  execute_cmd_pre_hook (stop_command);
+	}
+      CATCH (ex, RETURN_MASK_ALL)
+	{
+	  exception_fprintf (gdb_stderr, ex,
+			     "Error while running hook_stop:\n");
+	}
+      END_CATCH
 
       /* If the stop hook resumes the target, then there's no point in
 	 trying to notify about the previous stop; its context is
@@ -9021,19 +9025,22 @@ restore_infcall_control_state (struct infcall_control_state *inf_status)
 
   if (target_has_stack)
     {
-      /* The point of catch_errors is that if the stack is clobbered,
+      /* The point of the try/catch is that if the stack is clobbered,
          walking the stack might encounter a garbage pointer and
          error() trying to dereference it.  */
-      if (catch_errors
-	  ([&] ()
-	     {
-	       return restore_selected_frame (&inf_status->selected_frame_id);
-	     },
-	   "Unable to restore previously selected frame:\n",
-	   RETURN_MASK_ERROR) == 0)
-	/* Error in restoring the selected frame.  Select the innermost
-	   frame.  */
-	select_frame (get_current_frame ());
+      TRY
+	{
+	  restore_selected_frame (&inf_status->selected_frame_id);
+	}
+      CATCH (ex, RETURN_MASK_ERROR)
+	{
+	  exception_fprintf (gdb_stderr, ex,
+			     "Unable to restore previously selected frame:\n");
+	  /* Error in restoring the selected frame.  Select the
+	     innermost frame.  */
+	  select_frame (get_current_frame ());
+	}
+      END_CATCH
     }
 
   xfree (inf_status);
diff --git a/gdb/main.c b/gdb/main.c
index 8408bfc..3dd08ce 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -1147,7 +1147,15 @@ captured_main (void *data)
      change - SET_TOP_LEVEL() - has been eliminated.  */
   while (1)
     {
-      catch_errors (captured_command_loop, "", RETURN_MASK_ALL);
+      TRY
+	{
+	  captured_command_loop ();
+	}
+      CATCH (ex, RETURN_MASK_ALL)
+	{
+	  exception_print (gdb_stderr, ex);
+	}
+      END_CATCH
     }
   /* No exit -- exit is through quit_command.  */
 }
diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
index 2b84904..10ee691 100644
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -1315,19 +1315,19 @@ find_objc_msgcall_submethod (int (*f) (CORE_ADDR, CORE_ADDR *),
 			     CORE_ADDR pc, 
 			     CORE_ADDR *new_pc)
 {
-  if (catch_errors ([&] ()
-		      {
-			if (f (pc, new_pc) == 0)
-			  return 1;
-			else
-			  return 0;
-		      },
-		    "Unable to determine target of "
-		    "Objective-C method call (ignoring):\n",
-		    RETURN_MASK_ALL) == 0) 
-    return 1;
-  else 
-    return 0;
+  TRY
+    {
+      if (f (pc, new_pc) == 0)
+	return 1;
+    }
+  CATCH (ex, RETURN_MASK_ALL)
+    {
+      exception_fprintf (gdb_stderr, ex,
+			 "Unable to determine target of "
+			 "Objective-C method call (ignoring):\n");
+    }
+
+  return 0;
 }
 
 int 
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 2d9085a..cf4aa5f 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -565,7 +565,7 @@ record_full_arch_list_cleanups (void *ignore)
    record the running message of inferior and set them to
    record_full_arch_list, and add it to record_full_list.  */
 
-static int
+static void
 record_full_message (struct regcache *regcache, enum gdb_signal signal)
 {
   int ret;
@@ -633,19 +633,24 @@ record_full_message (struct regcache *regcache, enum gdb_signal signal)
     record_full_list_release_first ();
   else
     record_full_insn_num++;
-
-  return 1;
 }
 
-static int
+static bool
 record_full_message_wrapper_safe (struct regcache *regcache,
 				  enum gdb_signal signal)
 {
-  return catch_errors ([&] ()
-		         {
-			   return record_full_message (regcache, signal);
-			 },
-		       "", RETURN_MASK_ALL);
+  TRY
+    {
+      record_full_message (regcache, signal);
+    }
+  CATCH (ex, RETURN_MASK_ALL)
+    {
+      exception_print (gdb_stderr, ex);
+      return false;
+    }
+  END_CATCH
+
+  return true;
 }
 
 /* Set to 1 if record_full_store_registers and record_full_xfer_partial
diff --git a/gdb/solib.c b/gdb/solib.c
index 0349c8f..e605e6b 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -760,12 +760,19 @@ update_solib_list (int from_tty)
 	 have not opened a symbol file, we may be able to get its
 	 symbols now!  */
       if (inf->attach_flag && symfile_objfile == NULL)
-	catch_errors ([&] ()
-		        {
-			  return ops->open_symbol_file_object (from_tty);
-			},
-		      "Error reading attached process's symbol file.\n",
-		      RETURN_MASK_ALL);
+	{
+	  TRY
+	    {
+	      ops->open_symbol_file_object (from_tty);
+	    }
+	  CATCH (ex, RETURN_MASK_ALL)
+	    {
+	      exception_fprintf (gdb_stderr, ex,
+				 "Error reading attached "
+				 "process's symbol file.\n");
+	    }
+	  END_CATCH
+	}
     }
 
   /* GDB and the inferior's dynamic linker each maintain their own
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index 6236bad..aae42bb 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -54,16 +54,9 @@ FILE *std_err;
 
 static int block_depth (struct block *);
 
-struct print_symbol_args
-  {
-    struct gdbarch *gdbarch;
-    struct symbol *symbol;
-    int depth;
-    struct ui_file *outfile;
-  };
-
-static int print_symbol (struct gdbarch *gdbarch, struct symbol *symbol,
-			 int depth, struct ui_file *outfile);
+static void print_symbol (struct gdbarch *gdbarch, struct symbol *symbol,
+			  int depth, struct ui_file *outfile);
+
 \f
 
 void
@@ -358,13 +351,16 @@ dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile)
 	     block, not any blocks from included symtabs.  */
 	  ALL_DICT_SYMBOLS (BLOCK_DICT (b), iter, sym)
 	    {
-	      catch_errors ([&] ()
-			      {
-				return print_symbol (gdbarch, sym, depth + 1,
-						     outfile);
-			      },
-		            "Error printing symbol:\n",
-			    RETURN_MASK_ERROR);
+	      TRY
+		{
+		  print_symbol (gdbarch, sym, depth + 1, outfile);
+		}
+	      CATCH (ex, RETURN_MASK_ERROR)
+		{
+		  exception_fprintf (gdb_stderr, ex,
+				     "Error printing symbol:\n");
+		}
+	      END_CATCH
 	    }
 	}
       fprintf_filtered (outfile, "\n");
@@ -515,10 +511,9 @@ maintenance_print_symbols (const char *args, int from_tty)
     }
 }
 
-/* Print symbol SYMBOL on OUTFILE.  DEPTH says how far to indent.
-   Returns 0 for error, 1 for success.  */
+/* Print symbol SYMBOL on OUTFILE.  DEPTH says how far to indent.  */
 
-static int
+static void
 print_symbol (struct gdbarch *gdbarch, struct symbol *symbol,
 	      int depth, struct ui_file *outfile)
 {
@@ -541,7 +536,6 @@ print_symbol (struct gdbarch *gdbarch, struct symbol *symbol,
 					    section->the_bfd_section));
       else
 	fprintf_filtered (outfile, "\n");
-      return 1;
     }
   if (SYMBOL_DOMAIN (symbol) == STRUCT_DOMAIN)
     {
@@ -689,7 +683,6 @@ print_symbol (struct gdbarch *gdbarch, struct symbol *symbol,
 	}
     }
   fprintf_filtered (outfile, "\n");
-  return 1;
 }
 
 static void
-- 
2.5.5



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [users/palves/catch_exceptions] [PATCH] zap catch_exceptions (Re: [RFA] Change exceptions.h functions to use gdb::function_view)
  2017-09-27 22:37     ` Pedro Alves
@ 2017-09-28  0:02       ` Pedro Alves
  2017-09-28  2:04         ` Tom Tromey
  0 siblings, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2017-09-28  0:02 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 09/27/2017 11:37 PM, Pedro Alves wrote:
> On 09/27/2017 09:25 PM, Tom Tromey wrote:
>>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>>
>> Pedro> I'm borderline about this.  I have to say that I question the value of
>> Pedro> catch_exceptions&co, over just using TRY/CATCH + a scoped_restore(current_uiout)
>> Pedro> in the try scope + printing the exception.  A TRY/CATCH is likely to be
>> Pedro> easier to understand and debug, I think.
>> [...]
>> Pedro> Did you consider this?
>>
>> Nope.  This was actually just a preparatory patch for another const change.
>>
>> I can look into the change though.  I agree it would be better.
> 
> I took a stab at this.  I converted catch_errors to see what it
> looks like.  See below a(n untested) patch on top of yours.  

Alright, I've pushed this patch below on the branch as well.  It
gets rid of catch_exceptions / catch_exceptions_with_msg.  The
latter is done mostly by getting rid of the remaining vestigial libgdb
wrapper functions, which are really pointless nowadays.  This results in
a good number of simplifications.

(I checked that Insight doesn't use those functions.)

The branch builds, but it still untested.

From 2a015c47e1a4a9b117d54f357dc3540326811748 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 27 Sep 2017 23:37:48 +0100
Subject: [PATCH] zap catch_exceptions

---
 gdb/breakpoint.c      | 35 ++------------------
 gdb/breakpoint.h      |  3 ++
 gdb/exceptions.c      | 81 -----------------------------------------------
 gdb/gdb.h             | 58 ---------------------------------
 gdb/gdbthread.h       |  2 ++
 gdb/mi/mi-cmd-break.c | 13 ++++++--
 gdb/mi/mi-cmd-catch.c |  1 -
 gdb/mi/mi-interp.c    | 72 +++++++++++++++++++----------------------
 gdb/mi/mi-main.c      | 49 ++++++++++++++++------------
 gdb/remote-fileio.c   | 37 +++++++++-------------
 gdb/symfile-mem.c     | 61 +++++++++++------------------------
 gdb/thread.c          | 88 ++-------------------------------------------------
 12 files changed, 115 insertions(+), 385 deletions(-)
 delete mode 100644 gdb/gdb.h

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 02e028c..a7915fb 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -44,7 +44,6 @@
 #include "source.h"
 #include "linespec.h"
 #include "completer.h"
-#include "gdb.h"
 #include "ui-out.h"
 #include "cli/cli-script.h"
 #include "block.h"
@@ -6596,39 +6595,11 @@ breakpoint_address_bits (struct breakpoint *b)
   return print_address_bits;
 }
 
-static int
-do_captured_breakpoint_query (int bnum)
+void
+print_breakpoint (breakpoint *b)
 {
-  struct breakpoint *b;
   struct bp_location *dummy_loc = NULL;
-
-  ALL_BREAKPOINTS (b)
-    {
-      if (bnum == b->number)
-	{
-	  print_one_breakpoint (b, &dummy_loc, 0);
-	  return GDB_RC_OK;
-	}
-    }
-  return GDB_RC_NONE;
-}
-
-enum gdb_rc
-gdb_breakpoint_query (struct ui_out *uiout, int bnum, 
-		      char **error_message)
-{
-  /* For the moment we don't trust print_one_breakpoint() to not throw
-     an error.  */
-  if (catch_exceptions_with_msg
-      (uiout,
-       [&] (struct ui_out *)
-         {
-	   return do_captured_breakpoint_query (bnum);
-	 },
-       error_message, RETURN_MASK_ALL) < 0)
-    return GDB_RC_FAIL;
-  else
-    return GDB_RC_OK;
+  print_one_breakpoint (b, &dummy_loc, 0);
 }
 
 /* Return true if this breakpoint was set by the user, false if it is
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index ff49cd2..143eae3 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1644,4 +1644,7 @@ extern const char *ep_parse_optional_if_clause (const char **arg);
    UIOUT iff debugging multiple threads.  */
 extern void maybe_print_thread_hit_breakpoint (struct ui_out *uiout);
 
+/* Print the specified breakpoint.  */
+extern void print_breakpoint (breakpoint *bp);
+
 #endif /* !defined (BREAKPOINT_H) */
diff --git a/gdb/exceptions.c b/gdb/exceptions.c
index 97a56e6..0fc2de5 100644
--- a/gdb/exceptions.c
+++ b/gdb/exceptions.c
@@ -134,87 +134,6 @@ exception_fprintf (struct ui_file *file, struct gdb_exception e,
     }
 }
 
-/* Call FUNC(UIOUT, FUNC_ARGS) but wrapped within an exception
-   handler.  If an exception (enum return_reason) is thrown using
-   throw_exception() than all cleanups installed since
-   catch_exceptions() was entered are invoked, the (-ve) exception
-   value is then returned by catch_exceptions.  If FUNC() returns
-   normally (with a positive or zero return value) then that value is
-   returned by catch_exceptions().  It is an internal_error() for
-   FUNC() to return a negative value.
-
-   See exceptions.h for further usage details.  */
-
-/* MAYBE: cagney/1999-11-05: catch_errors() in conjunction with
-   error() et al. could maintain a set of flags that indicate the
-   current state of each of the longjmp buffers.  This would give the
-   longjmp code the chance to detect a longjmp botch (before it gets
-   to longjmperror()).  Prior to 1999-11-05 this wasn't possible as
-   code also randomly used a SET_TOP_LEVEL macro that directly
-   initialized the longjmp buffers.  */
-
-int
-catch_exceptions (struct ui_out *uiout,
-		  gdb::function_view<catch_exceptions_ftype> func,
-		  return_mask mask)
-{
-  return catch_exceptions_with_msg (uiout, func, NULL, mask);
-}
-
-int
-catch_exceptions_with_msg (struct ui_out *func_uiout,
-			   gdb::function_view<catch_exceptions_ftype> func,
-			   char **gdberrmsg,
-		  	   return_mask mask)
-{
-  struct gdb_exception exception = exception_none;
-  volatile int val = 0;
-  struct ui_out *saved_uiout;
-
-  /* Save and override the global ``struct ui_out'' builder.  */
-  saved_uiout = current_uiout;
-  current_uiout = func_uiout;
-
-  TRY
-    {
-      val = func (current_uiout);
-    }
-  CATCH (ex, RETURN_MASK_ALL)
-    {
-      exception = ex;
-    }
-  END_CATCH
-
-  /* Restore the global builder.  */
-  current_uiout = saved_uiout;
-
-  if (exception.reason < 0 && (mask & RETURN_MASK (exception.reason)) == 0)
-    {
-      /* The caller didn't request that the event be caught.
-	 Rethrow.  */
-      throw_exception (exception);
-    }
-
-  exception_print (gdb_stderr, exception);
-  gdb_assert (val >= 0);
-  gdb_assert (exception.reason <= 0);
-  if (exception.reason < 0)
-    {
-      /* If caller wants a copy of the low-level error message, make
-	 one.  This is used in the case of a silent error whereby the
-	 caller may optionally want to issue the message.  */
-      if (gdberrmsg != NULL)
-	{
-	  if (exception.message != NULL)
-	    *gdberrmsg = xstrdup (exception.message);
-	  else
-	    *gdberrmsg = NULL;
-	}
-      return exception.reason;
-    }
-  return val;
-}
-
 /* See exceptions.h.  */
 
 int
diff --git a/gdb/gdb.h b/gdb/gdb.h
deleted file mode 100644
index ac1e683..0000000
--- a/gdb/gdb.h
+++ /dev/null
@@ -1,58 +0,0 @@
-/* Library interface into GDB.
-   Copyright (C) 1999-2017 Free Software Foundation, Inc.
-
-   This file is part of GDB.
-
-   This program is free software; you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation; either version 3 of the License, or
-   (at your option) any later version.
-
-   This program is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
-
-#ifndef GDB_H
-#define GDB_H
-
-struct ui_out;
-
-/* Return-code (RC) from a gdb library call.  (The abreviation RC is
-   taken from the sim/common directory.) */
-
-enum gdb_rc {
-  /* The operation failed.  The failure message can be fetched by
-     calling ``char *error_last_message(void)''.  The value is
-     determined by the catch_errors() interface.  The MSG parameter is
-     set to a freshly allocated copy of the error message.  */
-  /* NOTE: Since ``defs.h:catch_errors()'' does not return an error /
-     internal / quit indication it is not possible to return that
-     here.  */
-  GDB_RC_FAIL = 0,
-  /* No error occured but nothing happened.  Due to the catch_errors()
-     interface, this must be non-zero.  */
-  GDB_RC_NONE = 1,
-  /* The operation was successful.  Due to the catch_errors()
-     interface, this must be non-zero.  */
-  GDB_RC_OK = 2
-};
-
-
-/* Print the specified breakpoint on GDB_STDOUT.  (Eventually this
-   function will ``print'' the object on ``output'').  */
-enum gdb_rc gdb_breakpoint_query (struct ui_out *uiout, int bnum,
-				  char **error_message);
-
-/* Switch thread and print notification.  */
-enum gdb_rc gdb_thread_select (struct ui_out *uiout, char *tidstr,
-			       char **error_message);
-
-/* Print a list of known thread ids.  */
-enum gdb_rc gdb_list_thread_ids (struct ui_out *uiout,
-				 char **error_message);
-
-#endif
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index d5101ab..9720993 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -679,6 +679,8 @@ extern int show_thread_that_caused_stop (void);
 extern void print_selected_thread_frame (struct ui_out *uiout,
 					 user_selected_what selection);
 
+extern void thread_select (const char *tidstr, thread_info *thr);
+
 extern struct thread_info *thread_list;
 
 #endif /* GDBTHREAD_H */
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index 188e4e2..c313c03 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -24,7 +24,6 @@
 #include "mi-out.h"
 #include "breakpoint.h"
 #include "mi-getopt.h"
-#include "gdb.h"
 #include "observer.h"
 #include "mi-main.h"
 #include "mi-cmd-break.h"
@@ -53,7 +52,17 @@ static void
 breakpoint_notify (struct breakpoint *b)
 {
   if (mi_can_breakpoint_notify)
-    gdb_breakpoint_query (current_uiout, b->number, NULL);
+    {
+      TRY
+	{
+	  print_breakpoint (b);
+	}
+      CATCH (ex, RETURN_MASK_ALL)
+	{
+	  exception_print (gdb_stdout, ex);
+	}
+      END_CATCH
+    }
 }
 
 enum bp_type
diff --git a/gdb/mi/mi-cmd-catch.c b/gdb/mi/mi-cmd-catch.c
index 0e1fb7e..9c103d2 100644
--- a/gdb/mi/mi-cmd-catch.c
+++ b/gdb/mi/mi-cmd-catch.c
@@ -21,7 +21,6 @@
 #include "defs.h"
 #include "arch-utils.h"
 #include "breakpoint.h"
-#include "gdb.h"
 #include "ada-lang.h"
 #include "mi-cmds.h"
 #include "mi-getopt.h"
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 714bb4b..20b5a71 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -33,7 +33,6 @@
 #include "observer.h"
 #include "gdbthread.h"
 #include "solist.h"
-#include "gdb.h"
 #include "objfiles.h"
 #include "tracepoint.h"
 #include "cli-out.h"
@@ -828,6 +827,36 @@ mi_tsv_modified (const struct trace_state_variable *tsv)
     }
 }
 
+static void
+mi_print_breakpoint (struct mi_interp *mi, breakpoint *bp)
+{
+  ui_out *mi_uiout = interp_ui_out (mi);
+
+  /* We want the output from print_breakpoint to go to
+     mi->event_channel.  One approach would be to just call
+     print_breakpoint, and then use mi_out_put to send the current
+     content of mi_uiout into mi->event_channel.  However, that will
+     break if anything is output to mi_uiout prior to calling the
+     breakpoint_created notifications.  So, we use
+     ui_out_redirect.  */
+  mi_uiout->redirect (mi->event_channel);
+
+  TRY
+    {
+      scoped_restore restore_uiout
+	= make_scoped_restore (&current_uiout, mi_uiout);
+
+      print_breakpoint (bp);
+    }
+  CATCH (ex, RETURN_MASK_ALL)
+    {
+      exception_print (gdb_stderr, ex);
+    }
+  END_CATCH
+
+  mi_uiout->redirect (NULL);
+}
+
 /* Emit notification about a created breakpoint.  */
 
 static void
@@ -842,36 +871,16 @@ mi_breakpoint_created (struct breakpoint *b)
   SWITCH_THRU_ALL_UIS ()
     {
       struct mi_interp *mi = as_mi_interp (top_level_interpreter ());
-      struct ui_out *mi_uiout;
 
       if (mi == NULL)
 	continue;
 
-      mi_uiout = interp_ui_out (top_level_interpreter ());
-
       target_terminal::scoped_restore_terminal_state term_state;
       target_terminal::ours_for_output ();
 
       fprintf_unfiltered (mi->event_channel,
 			  "breakpoint-created");
-      /* We want the output from gdb_breakpoint_query to go to
-	 mi->event_channel.  One approach would be to just call
-	 gdb_breakpoint_query, and then use mi_out_put to send the current
-	 content of mi_uiout into mi->event_channel.  However, that will
-	 break if anything is output to mi_uiout prior to calling the
-	 breakpoint_created notifications.  So, we use
-	 ui_out_redirect.  */
-      mi_uiout->redirect (mi->event_channel);
-      TRY
-	{
-	  gdb_breakpoint_query (mi_uiout, b->number, NULL);
-	}
-      CATCH (e, RETURN_MASK_ERROR)
-	{
-	}
-      END_CATCH
-
-      mi_uiout->redirect (NULL);
+      mi_print_breakpoint (mi, b);
 
       gdb_flush (mi->event_channel);
     }
@@ -927,24 +936,7 @@ mi_breakpoint_modified (struct breakpoint *b)
       target_terminal::ours_for_output ();
       fprintf_unfiltered (mi->event_channel,
 			  "breakpoint-modified");
-      /* We want the output from gdb_breakpoint_query to go to
-	 mi->event_channel.  One approach would be to just call
-	 gdb_breakpoint_query, and then use mi_out_put to send the current
-	 content of mi_uiout into mi->event_channel.  However, that will
-	 break if anything is output to mi_uiout prior to calling the
-	 breakpoint_created notifications.  So, we use
-	 ui_out_redirect.  */
-      mi->mi_uiout->redirect (mi->event_channel);
-      TRY
-	{
-	  gdb_breakpoint_query (mi->mi_uiout, b->number, NULL);
-	}
-      CATCH (e, RETURN_MASK_ERROR)
-	{
-	}
-      END_CATCH
-
-      mi->mi_uiout->redirect (NULL);
+      mi_print_breakpoint (mi, b);
 
       gdb_flush (mi->event_channel);
     }
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 5e71949..1cf3b7f 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -38,7 +38,6 @@
 #include "gdbcore.h"		/* For write_memory().  */
 #include "value.h"
 #include "regcache.h"
-#include "gdb.h"
 #include "frame.h"
 #include "mi-main.h"
 #include "mi-common.h"
@@ -553,21 +552,17 @@ mi_cmd_target_flash_erase (const char *command, char **argv, int argc)
 void
 mi_cmd_thread_select (const char *command, char **argv, int argc)
 {
-  enum gdb_rc rc;
-  char *mi_error_message;
-  ptid_t previous_ptid = inferior_ptid;
-
   if (argc != 1)
     error (_("-thread-select: USAGE: threadnum."));
 
-  rc = gdb_thread_select (current_uiout, argv[0], &mi_error_message);
+  int num = value_as_long (parse_and_eval (argv[0]));
+  thread_info *thr = find_thread_global_id (num);
+  if (thr == NULL)
+    error (_("Thread ID %d not known."), num);
 
-  /* If thread switch did not succeed don't notify or print.  */
-  if (rc == GDB_RC_FAIL)
-    {
-      make_cleanup (xfree, mi_error_message);
-      error ("%s", mi_error_message);
-    }
+  ptid_t previous_ptid = inferior_ptid;
+
+  thread_select (argv[0], thr);
 
   print_selected_thread_frame (current_uiout,
 			       USER_SELECTED_THREAD | USER_SELECTED_FRAME);
@@ -583,19 +578,31 @@ mi_cmd_thread_select (const char *command, char **argv, int argc)
 void
 mi_cmd_thread_list_ids (const char *command, char **argv, int argc)
 {
-  enum gdb_rc rc;
-  char *mi_error_message;
-
   if (argc != 0)
     error (_("-thread-list-ids: No arguments required."));
 
-  rc = gdb_list_thread_ids (current_uiout, &mi_error_message);
+  int num = 0;
+  int current_thread = -1;
 
-  if (rc == GDB_RC_FAIL)
-    {
-      make_cleanup (xfree, mi_error_message);
-      error ("%s", mi_error_message);
-    }
+  update_thread_list ();
+
+  {
+    ui_out_emit_tuple tuple_emitter (current_uiout, "thread-ids");
+
+    struct thread_info *tp;
+    ALL_NON_EXITED_THREADS (tp)
+      {
+	if (tp->ptid == inferior_ptid)
+	  current_thread = tp->global_num;
+
+	num++;
+	current_uiout->field_int ("thread-id", tp->global_num);
+      }
+  }
+
+  if (current_thread != -1)
+    current_uiout->field_int ("current-thread-id", current_thread);
+  current_uiout->field_int ("number-of-threads", num);
 }
 
 void
diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c
index f3e33ec..9d1a923 100644
--- a/gdb/remote-fileio.c
+++ b/gdb/remote-fileio.c
@@ -1118,10 +1118,9 @@ static struct {
   { NULL, NULL }
 };
 
-static int
-do_remote_fileio_request (struct ui_out *uiout, void *buf_arg)
+static void
+do_remote_fileio_request (char *buf)
 {
-  char *buf = (char *) buf_arg;
   char *c;
   int idx;
 
@@ -1135,10 +1134,10 @@ do_remote_fileio_request (struct ui_out *uiout, void *buf_arg)
   for (idx = 0; remote_fio_func_map[idx].name; ++idx)
     if (!strcmp (remote_fio_func_map[idx].name, buf))
       break;
-  if (!remote_fio_func_map[idx].name)	/* ERROR: No such function.  */
-    return RETURN_ERROR;
-  remote_fio_func_map[idx].func (c);
-  return 0;
+  if (!remote_fio_func_map[idx].name)
+    remote_fileio_reply (-1, FILEIO_ENOSYS);
+  else
+    remote_fio_func_map[idx].func (c);
 }
 
 /* Close any open descriptors, and reinitialize the file mapping.  */
@@ -1188,22 +1187,16 @@ remote_fileio_request (char *buf, int ctrlc_pending_p)
     }
   else
     {
-      ex = catch_exceptions (current_uiout,
-			     [&] (ui_out *uiout)
-			       {
-				 return do_remote_fileio_request (uiout, buf);
-			       },
-			     RETURN_MASK_ALL);
-      switch (ex)
+      TRY
 	{
-	case RETURN_ERROR:
-	  remote_fileio_reply (-1, FILEIO_ENOSYS);
-	  break;
-	case RETURN_QUIT:
-	  remote_fileio_reply (-1, FILEIO_EINTR);
-	  break;
-	default:
-	  break;
+	  do_remote_fileio_request (buf);
+	}
+      CATCH (ex, RETURN_MASK_ALL)
+	{
+	  if (ex.reason == RETURN_QUIT)
+	    remote_fileio_reply (-1, FILEIO_EINTR);
+	  else
+	    remote_fileio_reply (-1, FILEIO_EIO);
 	}
     }
 
diff --git a/gdb/symfile-mem.c b/gdb/symfile-mem.c
index 5cefbc9..c7ef013 100644
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -167,31 +167,6 @@ add_symbol_file_from_memory_command (const char *args, int from_tty)
   symbol_file_add_from_memory (templ, addr, 0, NULL, from_tty);
 }
 
-/* Arguments for symbol_file_add_from_memory_wrapper.  */
-
-struct symbol_file_add_from_memory_args
-{
-  struct bfd *bfd;
-  CORE_ADDR sysinfo_ehdr;
-  size_t size;
-  char *name;
-  int from_tty;
-};
-
-/* Wrapper function for symbol_file_add_from_memory, for
-   catch_exceptions.  */
-
-static int
-symbol_file_add_from_memory_wrapper (struct ui_out *uiout, void *data)
-{
-  struct symbol_file_add_from_memory_args *args
-    = (struct symbol_file_add_from_memory_args *) data;
-
-  symbol_file_add_from_memory (args->bfd, args->sysinfo_ehdr, args->size,
-			       args->name, args->from_tty);
-  return 0;
-}
-
 /* Try to add the symbols for the vsyscall page, if there is one.
    This function is called via the inferior_created observer.  */
 
@@ -203,7 +178,6 @@ add_vsyscall_page (struct target_ops *target, int from_tty)
   if (gdbarch_vsyscall_range (target_gdbarch (), &vsyscall_range))
     {
       struct bfd *bfd;
-      struct symbol_file_add_from_memory_args args;
 
       if (core_bfd != NULL)
 	bfd = core_bfd;
@@ -221,23 +195,24 @@ add_vsyscall_page (struct target_ops *target, int from_tty)
 		     "because no executable was specified"));
 	  return;
 	}
-      args.bfd = bfd;
-      args.sysinfo_ehdr = vsyscall_range.start;
-      args.size = vsyscall_range.length;
-
-      args.name = xstrprintf ("system-supplied DSO at %s",
-			      paddress (target_gdbarch (), vsyscall_range.start));
-      /* Pass zero for FROM_TTY, because the action of loading the
-	 vsyscall DSO was not triggered by the user, even if the user
-	 typed "run" at the TTY.  */
-      args.from_tty = 0;
-      catch_exceptions (current_uiout,
-			[&] (ui_out *uiout)
-			  {
-			    return symbol_file_add_from_memory_wrapper (uiout,
-									&args);
-			  },
-			RETURN_MASK_ALL);
+
+      char *name = xstrprintf ("system-supplied DSO at %s",
+			       paddress (target_gdbarch (), vsyscall_range.start));
+      TRY
+	{
+	  /* Pass zero for FROM_TTY, because the action of loading the
+	     vsyscall DSO was not triggered by the user, even if the
+	     user typed "run" at the TTY.  */
+	  symbol_file_add_from_memory (bfd,
+				       vsyscall_range.start,
+				       vsyscall_range.length,
+				       name,
+				       0 /* from_tty */);
+	}
+      CATCH (ex, RETURN_MASK_ALL)
+	{
+	  exception_print (gdb_stderr, ex);
+	}
     }
 }
 
diff --git a/gdb/thread.c b/gdb/thread.c
index 7deebf2..33c98e4 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -30,7 +30,6 @@
 #include "command.h"
 #include "gdbcmd.h"
 #include "regcache.h"
-#include "gdb.h"
 #include "btrace.h"
 
 #include <ctype.h>
@@ -720,50 +719,6 @@ any_live_thread_of_process (int pid)
   return tp_executing;
 }
 
-/* Print a list of thread ids currently known, and the total number of
-   threads.  To be used from within catch_errors.  */
-static int
-do_captured_list_thread_ids (struct ui_out *uiout)
-{
-  struct thread_info *tp;
-  int num = 0;
-  int current_thread = -1;
-
-  update_thread_list ();
-
-  {
-    ui_out_emit_tuple tuple_emitter (uiout, "thread-ids");
-
-    for (tp = thread_list; tp; tp = tp->next)
-      {
-	if (tp->state == THREAD_EXITED)
-	  continue;
-
-	if (tp->ptid == inferior_ptid)
-	  current_thread = tp->global_num;
-
-	num++;
-	uiout->field_int ("thread-id", tp->global_num);
-      }
-  }
-
-  if (current_thread != -1)
-    uiout->field_int ("current-thread-id", current_thread);
-  uiout->field_int ("number-of-threads", num);
-  return GDB_RC_OK;
-}
-
-/* Official gdblib interface function to get a list of thread ids and
-   the total number.  */
-enum gdb_rc
-gdb_list_thread_ids (struct ui_out *uiout, char **error_message)
-{
-  if (catch_exceptions_with_msg (uiout, do_captured_list_thread_ids,
-				 error_message, RETURN_MASK_ALL) < 0)
-    return GDB_RC_FAIL;
-  return GDB_RC_OK;
-}
-
 /* Return true if TP is an active thread.  */
 static int
 thread_alive (struct thread_info *tp)
@@ -1885,13 +1840,8 @@ thread_command (char *tidstr, int from_tty)
   else
     {
       ptid_t previous_ptid = inferior_ptid;
-      enum gdb_rc result;
-
-      result = gdb_thread_select (current_uiout, tidstr, NULL);
 
-      /* If thread switch did not succeed don't notify or print.  */
-      if (result == GDB_RC_FAIL)
-	return;
+      thread_select (tidstr, parse_thread_id (tidstr, NULL));
 
       /* Print if the thread has not changed, otherwise an event will
 	 be sent.  */
@@ -1991,25 +1941,9 @@ show_print_thread_events (struct ui_file *file, int from_tty,
 		    value);
 }
 
-static int
-do_captured_thread_select (struct ui_out *uiout, const char *tidstr)
+void
+thread_select (const char *tidstr, thread_info *tp)
 {
-  struct thread_info *tp;
-
-  if (uiout->is_mi_like_p ())
-    {
-      int num = value_as_long (parse_and_eval (tidstr));
-
-      tp = find_thread_global_id (num);
-      if (tp == NULL)
-	error (_("Thread ID %d not known."), num);
-    }
-  else
-    {
-      tp = parse_thread_id (tidstr, NULL);
-      gdb_assert (tp != NULL);
-    }
-
   if (!thread_alive (tp))
     error (_("Thread ID %s has terminated."), tidstr);
 
@@ -2020,8 +1954,6 @@ do_captured_thread_select (struct ui_out *uiout, const char *tidstr)
   /* Since the current thread may have changed, see if there is any
      exited thread we can now delete.  */
   prune_threads ();
-
-  return GDB_RC_OK;
 }
 
 /* Print thread and frame switch command response.  */
@@ -2066,20 +1998,6 @@ print_selected_thread_frame (struct ui_out *uiout,
     }
 }
 
-enum gdb_rc
-gdb_thread_select (struct ui_out *uiout, char *tidstr, char **error_message)
-{
-  if (catch_exceptions_with_msg
-      (uiout,
-       [&] (struct ui_out *inner_uiout)
-         {
-	   return do_captured_thread_select (inner_uiout, tidstr);
-	 },
-       error_message, RETURN_MASK_ALL) < 0)
-    return GDB_RC_FAIL;
-  return GDB_RC_OK;
-}
-
 /* Update the 'threads_executing' global based on the threads we know
    about right now.  */
 
-- 
2.5.5



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [users/palves/catch_exceptions] [PATCH] zap catch_exceptions (Re: [RFA] Change exceptions.h functions to use gdb::function_view)
  2017-09-28  0:02       ` [users/palves/catch_exceptions] [PATCH] zap catch_exceptions (Re: [RFA] Change exceptions.h functions to use gdb::function_view) Pedro Alves
@ 2017-09-28  2:04         ` Tom Tromey
  2017-09-28  9:13           ` Pedro Alves
  0 siblings, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2017-09-28  2:04 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

Pedro> Alright, I've pushed this patch below on the branch as well.  It
Pedro> gets rid of catch_exceptions / catch_exceptions_with_msg.  The
Pedro> latter is done mostly by getting rid of the remaining vestigial libgdb
Pedro> wrapper functions, which are really pointless nowadays.  This results in
Pedro> a good number of simplifications.

Thanks for doing this.  I'd started earlier, and did the catch_errors
part, but I saw your mail before I finished; and I think your patch is a
bit cleaner anyway.

I think my MI cleanup branch (still pending review) also touched these
vestigial libgdb functions, but I don't mind rebasing that when the time
comes.

Tom


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [users/palves/catch_exceptions] [PATCH] zap catch_exceptions (Re: [RFA] Change exceptions.h functions to use gdb::function_view)
  2017-09-28  2:04         ` Tom Tromey
@ 2017-09-28  9:13           ` Pedro Alves
  2017-09-28 19:53             ` Tom Tromey
  0 siblings, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2017-09-28  9:13 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 09/28/2017 03:04 AM, Tom Tromey wrote:
> Pedro> Alright, I've pushed this patch below on the branch as well.  It
> Pedro> gets rid of catch_exceptions / catch_exceptions_with_msg.  The
> Pedro> latter is done mostly by getting rid of the remaining vestigial libgdb
> Pedro> wrapper functions, which are really pointless nowadays.  This results in
> Pedro> a good number of simplifications.
> 
> Thanks for doing this.  I'd started earlier, and did the catch_errors
> part, but I saw your mail before I finished; and I think your patch is a
> bit cleaner anyway.

Sorry about that.  :-/  I only did it because I thought you wouldn't
be able to...  Here's what happened on IRC:

 <tromey> thanks for the review palves 
 <tromey> it may be a while before I get to that [snip]
 <-- tromey has left this server
 <palves> tromey: ack.  I'm tired so I'm using that as procrastination :-P  i'm eliminating catch_errors using TRY/CATCH.  will show what it looks like in a bit.
 <palves> this exposes lots of cleanup opportunity.
 <palves> err, ENOTROMEY...

:-P

> I think my MI cleanup branch (still pending review) also touched these
> vestigial libgdb functions, but I don't mind rebasing that when the time
> comes.

I'll take a look at that.

Thanks,
Pedro Alves


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [users/palves/catch_exceptions] [PATCH] zap catch_exceptions (Re: [RFA] Change exceptions.h functions to use gdb::function_view)
  2017-09-28  9:13           ` Pedro Alves
@ 2017-09-28 19:53             ` Tom Tromey
  0 siblings, 0 replies; 8+ messages in thread
From: Tom Tromey @ 2017-09-28 19:53 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Sorry about that.  :-/  I only did it because I thought you wouldn't
Pedro> be able to...

Totally understandable and no big deal.  FWIW this patch was actually
preparation for a constification patch that is still waiting here to be
submitted.

Tom


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-09-28 19:53 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-27 16:53 [RFA] Change exceptions.h functions to use gdb::function_view Tom Tromey
2017-09-27 19:43 ` Pedro Alves
2017-09-27 20:26   ` Tom Tromey
2017-09-27 22:37     ` Pedro Alves
2017-09-28  0:02       ` [users/palves/catch_exceptions] [PATCH] zap catch_exceptions (Re: [RFA] Change exceptions.h functions to use gdb::function_view) Pedro Alves
2017-09-28  2:04         ` Tom Tromey
2017-09-28  9:13           ` Pedro Alves
2017-09-28 19:53             ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox