Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* FYI: small simplification in breakpoint.c
@ 2011-04-04 15:22 Tom Tromey
  2011-04-04 15:34 ` Pedro Alves
  0 siblings, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2011-04-04 15:22 UTC (permalink / raw)
  To: gdb-patches

I'm checking this in on the trunk.

This patch is just a little cleanup to replace a call to catch_exception
(and associated helper function and argument struct) with a direct use
of TRY_CATCH.

Built and regtested on x86-64 (compile farm).

Tom

2011-04-04  Tom Tromey  <tromey@redhat.com>

	* breakpoint.c (struct captured_parse_breakpoint_args): Remove.
	(do_captured_parse_breakpoint): Remove.
	(create_breakpoint): `e' is now volatile.  Remove `parse_args'.
	Use TRY_CATCH directly.

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.562
diff -u -r1.562 breakpoint.c
--- breakpoint.c	1 Apr 2011 16:59:57 -0000	1.562
+++ breakpoint.c	4 Apr 2011 15:21:05 -0000
@@ -6066,14 +6066,6 @@
       delete_breakpoint (b);
 }
 
-struct captured_parse_breakpoint_args
-  {
-    char **arg_p;
-    struct symtabs_and_lines *sals_p;
-    struct linespec_result *canonical_p;
-    int *not_found_ptr;
-  };
-
 struct lang_and_radix
   {
     enum language lang;
@@ -7774,15 +7766,6 @@
     }
 }
 
-static void
-do_captured_parse_breakpoint (struct ui_out *ui, void *data)
-{
-  struct captured_parse_breakpoint_args *args = data;
-  
-  parse_breakpoint_sals (args->arg_p, args->sals_p, args->canonical_p, 
-		         args->not_found_ptr);
-}
-
 /* Given TOK, a string specification of condition and thread, as
    accepted by the 'break' command, extract the condition
    string and thread number and set *COND_STRING and *THREAD.
@@ -7918,7 +7901,7 @@
 		   struct breakpoint_ops *ops,
 		   int from_tty, int enabled, int internal)
 {
-  struct gdb_exception e;
+  volatile struct gdb_exception e;
   struct symtabs_and_lines sals;
   struct symtab_and_line pending_sal;
   char *copy_arg;
@@ -7926,7 +7909,6 @@
   struct linespec_result canonical;
   struct cleanup *old_chain;
   struct cleanup *bkpt_chain = NULL;
-  struct captured_parse_breakpoint_args parse_args;
   int i;
   int pending = 0;
   int not_found = 0;
@@ -7937,11 +7919,6 @@
   sals.nelts = 0;
   init_linespec_result (&canonical);
 
-  parse_args.arg_p = &arg;
-  parse_args.sals_p = &sals;
-  parse_args.canonical_p = &canonical;
-  parse_args.not_found_ptr = &not_found;
-
   if (type_wanted == bp_static_tracepoint && is_marker_spec (arg))
     {
       int i;
@@ -7955,8 +7932,10 @@
       goto done;
     }
 
-  e = catch_exception (uiout, do_captured_parse_breakpoint, 
-		       &parse_args, RETURN_MASK_ALL);
+  TRY_CATCH (e, RETURN_MASK_ALL)
+    {
+      parse_breakpoint_sals (&arg, &sals, &canonical, &not_found);
+    }
 
   /* If caller is interested in rc value from parse, set value.  */
   switch (e.reason)


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

* Re: FYI: small simplification in breakpoint.c
  2011-04-04 15:22 FYI: small simplification in breakpoint.c Tom Tromey
@ 2011-04-04 15:34 ` Pedro Alves
  2011-04-04 15:39   ` Pedro Alves
  0 siblings, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2011-04-04 15:34 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

On Monday 04 April 2011 16:22:35, Tom Tromey wrote:
> I'm checking this in on the trunk.
> 
> This patch is just a little cleanup to replace a call to catch_exception
> (and associated helper function and argument struct) with a direct use
> of TRY_CATCH.

catch_exceptions also prints the exception string, if
any (w/ print_any_exception), was the change intentional?  (your note seems to
imply this is meant as a 1-1 replacement, but I do see a few
exception_print's below the TRY_CATCH, so I can't tell.)

> 
> Built and regtested on x86-64 (compile farm).
> 
> Tom
> 
> 2011-04-04  Tom Tromey  <tromey@redhat.com>
> 
> 	* breakpoint.c (struct captured_parse_breakpoint_args): Remove.
> 	(do_captured_parse_breakpoint): Remove.
> 	(create_breakpoint): `e' is now volatile.  Remove `parse_args'.
> 	Use TRY_CATCH directly.
> 
> Index: breakpoint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/breakpoint.c,v
> retrieving revision 1.562
> diff -u -r1.562 breakpoint.c
> --- breakpoint.c	1 Apr 2011 16:59:57 -0000	1.562
> +++ breakpoint.c	4 Apr 2011 15:21:05 -0000
> @@ -6066,14 +6066,6 @@
>        delete_breakpoint (b);
>  }
>  
> -struct captured_parse_breakpoint_args
> -  {
> -    char **arg_p;
> -    struct symtabs_and_lines *sals_p;
> -    struct linespec_result *canonical_p;
> -    int *not_found_ptr;
> -  };
> -
>  struct lang_and_radix
>    {
>      enum language lang;
> @@ -7774,15 +7766,6 @@
>      }
>  }
>  
> -static void
> -do_captured_parse_breakpoint (struct ui_out *ui, void *data)
> -{
> -  struct captured_parse_breakpoint_args *args = data;
> -  
> -  parse_breakpoint_sals (args->arg_p, args->sals_p, args->canonical_p, 
> -		         args->not_found_ptr);
> -}
> -
>  /* Given TOK, a string specification of condition and thread, as
>     accepted by the 'break' command, extract the condition
>     string and thread number and set *COND_STRING and *THREAD.
> @@ -7918,7 +7901,7 @@
>  		   struct breakpoint_ops *ops,
>  		   int from_tty, int enabled, int internal)
>  {
> -  struct gdb_exception e;
> +  volatile struct gdb_exception e;
>    struct symtabs_and_lines sals;
>    struct symtab_and_line pending_sal;
>    char *copy_arg;
> @@ -7926,7 +7909,6 @@
>    struct linespec_result canonical;
>    struct cleanup *old_chain;
>    struct cleanup *bkpt_chain = NULL;
> -  struct captured_parse_breakpoint_args parse_args;
>    int i;
>    int pending = 0;
>    int not_found = 0;
> @@ -7937,11 +7919,6 @@
>    sals.nelts = 0;
>    init_linespec_result (&canonical);
>  
> -  parse_args.arg_p = &arg;
> -  parse_args.sals_p = &sals;
> -  parse_args.canonical_p = &canonical;
> -  parse_args.not_found_ptr = &not_found;
> -
>    if (type_wanted == bp_static_tracepoint && is_marker_spec (arg))
>      {
>        int i;
> @@ -7955,8 +7932,10 @@
>        goto done;
>      }
>  
> -  e = catch_exception (uiout, do_captured_parse_breakpoint, 
> -		       &parse_args, RETURN_MASK_ALL);
> +  TRY_CATCH (e, RETURN_MASK_ALL)
> +    {
> +      parse_breakpoint_sals (&arg, &sals, &canonical, &not_found);
> +    }
>  
>    /* If caller is interested in rc value from parse, set value.  */
>    switch (e.reason)
> 

-- 
Pedro Alves


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

* Re: FYI: small simplification in breakpoint.c
  2011-04-04 15:34 ` Pedro Alves
@ 2011-04-04 15:39   ` Pedro Alves
  2011-04-04 15:42     ` Tom Tromey
  0 siblings, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2011-04-04 15:39 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

On Monday 04 April 2011 16:33:37, Pedro Alves wrote:
> catch_exceptions also prints the exception string, if
> any (w/ print_any_exception), was the change intentional?  (your note seems to
> imply this is meant as a 1-1 replacement, but I do see a few
> exception_print's below the TRY_CATCH, so I can't tell.)

Oh, sorry.  Please ignore me.  That's catch_exceptionS, but you
replaced a use of catch_exception, which doesn't do that.

-- 
Pedro Alves


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

* Re: FYI: small simplification in breakpoint.c
  2011-04-04 15:39   ` Pedro Alves
@ 2011-04-04 15:42     ` Tom Tromey
  2011-04-04 15:44       ` Pedro Alves
  0 siblings, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2011-04-04 15:42 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> On Monday 04 April 2011 16:33:37, Pedro Alves wrote:
>> catch_exceptions also prints the exception string, if
>> any (w/ print_any_exception), was the change intentional?  (your note seems to
>> imply this is meant as a 1-1 replacement, but I do see a few
>> exception_print's below the TRY_CATCH, so I can't tell.)

Pedro> Oh, sorry.  Please ignore me.  That's catch_exceptionS, but you
Pedro> replaced a use of catch_exception, which doesn't do that.

FWIW, I'm not a fan of having two functions with such similar names.
This has bitten me a couple of times.

Tom


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

* Re: FYI: small simplification in breakpoint.c
  2011-04-04 15:42     ` Tom Tromey
@ 2011-04-04 15:44       ` Pedro Alves
  2011-04-04 16:14         ` Tom Tromey
  0 siblings, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2011-04-04 15:44 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On Monday 04 April 2011 16:41:31, Tom Tromey wrote:
> >>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
> 
> Pedro> On Monday 04 April 2011 16:33:37, Pedro Alves wrote:
> >> catch_exceptions also prints the exception string, if
> >> any (w/ print_any_exception), was the change intentional?  (your note seems to
> >> imply this is meant as a 1-1 replacement, but I do see a few
> >> exception_print's below the TRY_CATCH, so I can't tell.)
> 
> Pedro> Oh, sorry.  Please ignore me.  That's catch_exceptionS, but you
> Pedro> replaced a use of catch_exception, which doesn't do that.
> 
> FWIW, I'm not a fan of having two functions with such similar names.
> This has bitten me a couple of times.

On my current checkout I see:

 $ grep "= catch_exception (" * -rn
 breakpoint.c:7958:  e = catch_exception (uiout, do_captured_parse_breakpoint, 
 cli/cli-script.c:1632:    e = catch_exception (uiout, wrapped_read_command_file, &args,
 cli/cli-interp.c:137:  e = catch_exception (uiout, do_captured_execute_command, &args,
 mi/mi-main.c:1962:      result = catch_exception (uiout, captured_mi_execute_command, command,
 remote.c:4070:    ex = catch_exception (uiout, remote_start_remote, &args, RETURN_MASK_ALL);

How about we just delete catch_exception?  It doesn't seem to add any value.

-- 
Pedro Alves


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

* Re: FYI: small simplification in breakpoint.c
  2011-04-04 15:44       ` Pedro Alves
@ 2011-04-04 16:14         ` Tom Tromey
  0 siblings, 0 replies; 6+ messages in thread
From: Tom Tromey @ 2011-04-04 16:14 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> How about we just delete catch_exception?  It doesn't seem to add
Pedro> any value.

Works for me.  I'm testing a patch.

Tom


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

end of thread, other threads:[~2011-04-04 16:14 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-04 15:22 FYI: small simplification in breakpoint.c Tom Tromey
2011-04-04 15:34 ` Pedro Alves
2011-04-04 15:39   ` Pedro Alves
2011-04-04 15:42     ` Tom Tromey
2011-04-04 15:44       ` Pedro Alves
2011-04-04 16:14         ` Tom Tromey

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