FYI: add user_breakpoint_p

FYI: add user_breakpoint_p

[Tom Tromey]

I am planning to check this in.

I needed a `user_breakpoint_p' function for async breakpoint
notifications in MI.  While writing this, I noticed similar code in
breakpoint.c, so this patch introduces the function and changes
everyplace to use it.

There was some inconsistency about whether the test should be ">= 0"
or "> 0".  The latter seems more correct to me, since I think user
breakpoints always begin with 1.

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

Tom

2011-01-27  Tom Tromey  <tromey@redhat.com>

	* breakpoint.h (user_breakpoint_p): Declare.
	* breakpoint.c (user_breakpoint_p): New function.
	(breakpoint_1): Use it.
	(save_breakpoints): Likewise.

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 8d0692b..c7a6484 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5064,6 +5064,15 @@ user_settable_breakpoint (const struct breakpoint *b)
 	  || is_watchpoint (b));
 }
 
+/* Return true if this breakpoint was set by the user, false if it is
+   internal or momentary.  */
+
+int
+user_breakpoint_p (struct breakpoint *b)
+{
+  return user_settable_breakpoint (b) && b->number > 0;
+}
+
 /* Print information on user settable breakpoint (watchpoint, etc)
    number BNUM.  If BNUM is -1 print all user-settable breakpoints.
    If ALLFLAG is non-zero, include non-user-settable breakpoints.  If
@@ -5096,8 +5105,7 @@ breakpoint_1 (int bnum, int allflag,
 	if (filter && !filter (b))
 	  continue;
 	
-	if (allflag || (user_settable_breakpoint (b)
-			&& b->number > 0))
+	if (allflag || user_breakpoint_p (b))
 	  {
 	    int addr_bit, type_len;
 
@@ -5169,8 +5177,7 @@ breakpoint_1 (int bnum, int allflag,
 	
 	/* We only print out user settable breakpoints unless the
 	   allflag is set.  */
-	if (allflag || (user_settable_breakpoint (b)
-			&& b->number > 0))
+	if (allflag || user_breakpoint_p (b))
 	  print_one_breakpoint (b, &last_loc, print_address_bits, allflag);
       }
   }
@@ -11727,7 +11734,7 @@ save_breakpoints (char *filename, int from_tty,
   ALL_BREAKPOINTS (tp)
   {
     /* Skip internal and momentary breakpoints.  */
-    if (!user_settable_breakpoint (tp) || tp->number < 0)
+    if (! user_breakpoint_p (tp))
       continue;
 
     /* If we have a filter, only save the breakpoints it accepts.  */
@@ -11765,7 +11772,7 @@ save_breakpoints (char *filename, int from_tty,
   ALL_BREAKPOINTS (tp)
   {
     /* Skip internal and momentary breakpoints.  */
-    if (!user_settable_breakpoint (tp) || tp->number < 0)
+    if (! user_breakpoint_p (tp))
       continue;
 
     /* If we have a filter, only save the breakpoints it accepts.  */
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 69598a7..6eed2cd 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1184,4 +1184,6 @@ extern void end_rbreak_breakpoints (void);
 extern struct breakpoint *iterate_over_breakpoints (int (*) (struct breakpoint *,
 							     void *), void *);
 
+extern int user_breakpoint_p (struct breakpoint *);
+
 #endif /* !defined (BREAKPOINT_H) */

Re: FYI: add user_breakpoint_p

[Joel Brobecker]

> 2011-01-27  Tom Tromey  <tromey@redhat.com>
> 
> 	* breakpoint.h (user_breakpoint_p): Declare.
> 	* breakpoint.c (user_breakpoint_p): New function.
> 	(breakpoint_1): Use it.
> 	(save_breakpoints): Likewise.

I love these little cleanups :-).

Just one question about style:

> -    if (!user_settable_breakpoint (tp) || tp->number < 0)
> +    if (! user_breakpoint_p (tp))

Do we agree on whether to write this as the above, or whether we should
not have a space between the '!' and the expression being negated:

   if (! user_breakpoint_p (tp))

?

I did a rough count, and currently we have a bout 500 instance with the
space, and 2800 without...

-- 
Joel

Re: FYI: add user_breakpoint_p

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> Just one question about style:

>> -    if (!user_settable_breakpoint (tp) || tp->number < 0)
>> +    if (! user_breakpoint_p (tp))

Joel> Do we agree on whether to write this as the above, or whether we should
Joel> not have a space between the '!' and the expression being negated:

I have trouble remembering whether we have a rule here.
I will remove the space.  It doesn't matter to me either way if we have
it or not.

Tom

Re: FYI: add user_breakpoint_p

[Joel Brobecker]

> >> -    if (!user_settable_breakpoint (tp) || tp->number < 0)
> >> +    if (! user_breakpoint_p (tp))
> 
> Joel> Do we agree on whether to write this as the above, or whether we should
> Joel> not have a space between the '!' and the expression being negated:
> 
> I have trouble remembering whether we have a rule here.
> I will remove the space.  It doesn't matter to me either way if we have
> it or not.

It doesn't matter to me either way, althought I have a slight preference
for no space.  I'd like us to agree and make a "soft" recommendation on
my wiki page about the GDB Coding Style.

-- 
Joel

Re: FYI: add user_breakpoint_p

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> It doesn't matter to me either way, althought I have a slight preference
Joel> for no space.  I'd like us to agree and make a "soft" recommendation on
Joel> my wiki page about the GDB Coding Style.

Go for it.

Tom

Re: FYI: add user_breakpoint_p

[Pedro Alves]

On Tuesday 01 February 2011 03:05:44, Joel Brobecker wrote:
> > >> -    if (!user_settable_breakpoint (tp) || tp->number < 0)
> > >> +    if (! user_breakpoint_p (tp))
> > 
> > Joel> Do we agree on whether to write this as the above, or whether we should
> > Joel> not have a space between the '!' and the expression being negated:
> > 
> > I have trouble remembering whether we have a rule here.
> > I will remove the space.  It doesn't matter to me either way if we have
> > it or not.
> 
> It doesn't matter to me either way, althought I have a slight preference
> for no space.  I'd like us to agree and make a "soft" recommendation on
> my wiki page about the GDB Coding Style.

I still have in my queue to post a patch per:

 <http://sourceware.org/ml/gdb-patches/2010-11/msg00121.html>

If you guys are still okay with it, I'll bump it close to the
the top of my today's TODO.  (I'm okay with having this in
the wiki as well).

-- 
Pedro Alves

Re: FYI: add user_breakpoint_p

[Tom Tromey]

Pedro> I still have in my queue to post a patch per:
Pedro>  <http://sourceware.org/ml/gdb-patches/2010-11/msg00121.html>
Pedro> If you guys are still okay with it, I'll bump it close to the
Pedro> the top of my today's TODO.  (I'm okay with having this in
Pedro> the wiki as well).

It seems like a good idea to me.

Tom

Re: FYI: add user_breakpoint_p

[Joel Brobecker]

> Pedro> I still have in my queue to post a patch per:
> Pedro>  <http://sourceware.org/ml/gdb-patches/2010-11/msg00121.html>
> Pedro> If you guys are still okay with it, I'll bump it close to the
> Pedro> the top of my today's TODO.  (I'm okay with having this in
> Pedro> the wiki as well).
> 
> It seems like a good idea to me.

Me too.

-- 
Joel