Two identical "else if" blocks ?

Two identical "else if" blocks ?

[Joel Brobecker]

Hello,

While reviewing everyone comments regarding the patch for catching
Ada exceptions, I noticed the following code in remove_breakpoint:

  else if ((b->owner->type == bp_catch_catch || 
            b->owner->type == bp_catch_throw)
           && breakpoint_enabled (b->owner)
           && !b->duplicate)
    {
      val = target_remove_breakpoint (&b->target_info);
      if (val)
        return val;
      b->inserted = (is == mark_inserted);
    }
  else if (ep_is_exception_catchpoint (b->owner)
           && b->inserted       /* sometimes previous insert doesn't happen */
           && breakpoint_enabled (b->owner)
           && !b->duplicate)
    {
      val = target_remove_breakpoint (&b->target_info);
      if (val)
        return val;

      b->inserted = (is == mark_inserted);
    }

Since ep_is_exception_catchpoint is:

  return (ep->type == bp_catch_catch) || (ep->type == bp_catch_throw);

It looks like the two blocks are completely identical, and we should
be able to remove one of them. Should I perhaps test what happens
when I delete, say, the first one?

-- 
Joel

Re: Two identical "else if" blocks ?

[Daniel Jacobowitz]

On Mon, Jan 01, 2007 at 07:52:49AM +0400, Joel Brobecker wrote:
> It looks like the two blocks are completely identical, and we should
> be able to remove one of them. Should I perhaps test what happens
> when I delete, say, the first one?

They're not 100% the same (there's an extra b->inserted check) so I'd
recommend deleting the second one, since it's dead code.

Looks like they've been the same since at least the dawn of CVS.

-- 
Daniel Jacobowitz
CodeSourcery

[commit] (was: "Re: Two identical "else if" blocks ?")

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 554 bytes --]

> > It looks like the two blocks are completely identical, and we should
> > be able to remove one of them. Should I perhaps test what happens
> > when I delete, say, the first one?
> 
> They're not 100% the same (there's an extra b->inserted check) so I'd
> recommend deleting the second one, since it's dead code.

Argh! I knew I had a blind spot :-/.

I've checked the following change in after having tested in on x86-linux.

2006-01-01  Joel Brobecker  <brobecker@adacore.com>

       * breakpoint.c (remove_breakpoint): Remove dead code.

-- 
Joel

[-- Attachment #2: dead.diff --]
[-- Type: text/plain, Size: 753 bytes --]

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.234
diff -u -p -r1.234 breakpoint.c
--- breakpoint.c	18 Dec 2006 22:10:13 -0000	1.234
+++ breakpoint.c	1 Jan 2007 06:05:33 -0000
@@ -1637,17 +1637,6 @@ remove_breakpoint (struct bp_location *b
 	return val;
       b->inserted = (is == mark_inserted);
     }
-  else if (ep_is_exception_catchpoint (b->owner)
-	   && b->inserted	/* sometimes previous insert doesn't happen */
-	   && breakpoint_enabled (b->owner)
-	   && !b->duplicate)
-    {
-      val = target_remove_breakpoint (&b->target_info);
-      if (val)
-	return val;
-
-      b->inserted = (is == mark_inserted);
-    }
 
   return 0;
 }