Re: [Patch] Watchpoint condition fix

[Joel Brobecker <brobecker@adacore.com>]

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

You are right to keep it, but not for the reason you explained.
I forgot that parse_exp_1 advances the string pointer that it is given.
So if we didn't keep the temporary variable, we would screw b->cond_string
(oops!).

> 2008-04-17  Marc Khouzam  <marc.khouzam@ericsson.com>
> 
>        * breakpoint.c (update_watchpoint): Always reparse
>        condition.

This is OK after having fixed a tiny little detail:

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

Reformat the comment to avoid the '*' at the beginning of each line
and put the final '*/' at the end of the last line, not on a new line:

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

Thank you,
-- 
Joel