RE: [Patch] Watchpoint condition fix

["Marc Khouzam" <marc.khouzam@ericsson.com>]

> This part of the change is OK, but I would like to see a comment

Done

> 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. 

Done

> As a result,
> the temporary variable "s" shouldn't be necessary either.

I'm not sure about this one.  s is pointing to b->cond_string
and not b->loc->cond.  Is it really unecessary?

Here is the updated patch (having kept the temp var s)

Marc


2008-04-17  Marc Khouzam  <marc.khouzam@ericsson.com>

       * breakpoint.c (update_watchpoint): Always reparse
       condition.


Index: gdb/breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.308
diff -u -r1.308 breakpoint.c
--- gdb/breakpoint.c    15 Apr 2008 14:32:12 -0000      1.308
+++ gdb/breakpoint.c    17 Apr 2008 17:40:13 -0000
@@ -994,14 +994,14 @@
            value_free (v);
        }
 
-      if (reparse && b->cond_string != NULL)
+      /* We just regenerated the list of breakpoint locations.
+       * The new location does not have its condition field set to anything
+       * and therefore, we must always reparse the cond_string, independently
+       * of the value of the reparse flag.
+       */
+      if (b->cond_string != NULL)
        {
          char *s = b->cond_string;
-         if (b->loc->cond)
-           {
-             xfree (b->loc->cond);
-             b->loc->cond = NULL;
-           }
          b->loc->cond = parse_exp_1 (&s, b->exp_valid_block, 0);
        }
     }