Re: [Patch] Watchpoint condition fix

[Joel Brobecker <brobecker@adacore.com>]

> Watchpoint conditions are broken because they get deleted when a
> watchpoint gets re-inserted.  When watchpoints get re-inserted, the
> condition string should be always reparsed.

I agree with you and Vladimir.

> @@ -986,7 +986,7 @@
>             value_free (v);
>         }
>  
> -      if (reparse && b->cond_string != NULL)
> +      if (b->cond_string != NULL)

This part of the change is OK, but I would like to see a comment
explaining why we have to re-evaluate the condition regardless of
the value of reparse.  Vladimir's explaination on the gdb@ mailing
list pretty much covers it.

>         {
>           char *s = b->cond_string;
>           if (b->loc->cond)

Also, you can remove the block of code that checks b->loc->cond
and frees when non-NULL, since we've just re-created the bplocs
earlier and we know that their cond field is NULL. As a result,
the temporary variable "s" shouldn't be necessary either.

As an aside, but this is not your doing, I think we have a memory leak.
We appear to be doing an xfree of each bp_location of our watchpoint,
but instead we should call free_bp_location (). Vladimir, do you agree?
(I will take care of doing that if you agree)

Thank you,
-- 
Joel