Re: [RFA] deleting breakpoints inside of 'commands' [Repost]

[Michael Snyder <msnyder at cygnus dot com>]

Don Howard wrote:
> 
> [This is a repost with a few nits fixed, and addressed to the correct
> list =) ]
> 
> Here is an other variation on how to deal with 'commands' scripts that
> delete their own breakpoint.  The patch below makes a copy of the
> commands list before executing it and deletes the copy when finished.
> Comments?

You mean other than "the concept makes my head hurt"?   ;-)

Well, let's see... seems like making a copy unconditionally
is going to slow down the execution of all command lists, 
for the benefit of those few that delete themselves.  

Can we think of a way to make the duplication conditional
on the need?  That is, on the current command list being
deleted?  Put off the cost until it becomes necessary?

I can think of one method, but it would require an extra pointer
in the breakpoint struct.  "execute_control_command" would set
this pointer to point to its command list, and delete_breakpoint
would check the pointer.  If it's non-null, then the duplication
would be done by delete_breakpoint, and the pointer updated to 
point to the duplicate.

Is it worth the effort?  Is this duplication costly 
compared to everything else already being done by
bpstat_do_actions?  Or am I worrying over nothing?

Michael
 

> 
> 2001-09-14  Don Howard  <dhoward@redhat.com>
> 
>         * breakpoint.c (bpstat_do_actions): Avoid deleting a 'commands'
>         list while executing that list.  Thanks to Pierre Muller
>         <muller@ics.u-strasbg.fr> for suggesting this implementation.
>         * cli/cli-script.c (dup_command_lines): New function.
>         * defs.h: Added declaration of new function.
> 
> Index: breakpoint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/breakpoint.c,v
> retrieving revision 1.52
> diff -p -u -w -r1.52 breakpoint.c
> --- breakpoint.c        2001/08/02 11:58:28     1.52
> +++ breakpoint.c        2001/09/17 16:30:46
> @@ -1824,7 +1824,8 @@ top:
>    breakpoint_proceeded = 0;
>    for (; bs != NULL; bs = bs->next)
>      {
> -      cmd = bs->commands;
> +      cmd = dup_command_lines (bs->commands);
> +
>        while (cmd != NULL)
>         {
>           execute_control_command (cmd);
> @@ -1834,6 +1835,9 @@ top:
>           else
>             cmd = cmd->next;
>         }
> +
> +      free_command_lines (&cmd);
> +
>        if (breakpoint_proceeded)
>         /* The inferior is proceeded by the command; bomb out now.
>            The bpstat chain has been blown away by wait_for_inferior.
> Index: defs.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/defs.h,v
> retrieving revision 1.63
> diff -p -u -w -r1.63 defs.h
> --- defs.h      2001/09/07 21:33:08     1.63
> +++ defs.h      2001/09/17 16:30:47
> @@ -837,6 +837,7 @@ struct command_line
>  extern struct command_line *read_command_lines (char *, int);
> 
>  extern void free_command_lines (struct command_line **);
> +extern struct command_line * dup_command_lines (struct command_line *);
> 
>  /* To continue the execution commands when running gdb asynchronously.
>     A continuation structure contains a pointer to a function to be called
> Index: cli/cli-script.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/cli/cli-script.c,v
> retrieving revision 1.7
> diff -p -u -w -r1.7 cli-script.c
> --- cli-script.c        2001/06/17 15:16:12     1.7
> +++ cli-script.c        2001/09/17 16:30:47
> @@ -1005,6 +1005,59 @@ read_command_lines (char *prompt_arg, in
>    return (head);
>  }
> 
> +/* Duplicate a chain of struct command_line's */
> +
> +struct command_line *
> +dup_command_lines (struct command_line *l)
> +{
> +  struct command_line *dup = NULL;
> +  register struct command_line *next = NULL;
> +
> +
> +  for (; l ; l = l->next)
> +    {
> +      if (next == NULL)
> +       {
> +         dup = next = (struct command_line *)
> +           xmalloc (sizeof (struct command_line));
> +       }
> +      else
> +       {
> +         next->next = (struct command_line *)
> +           xmalloc (sizeof (struct command_line));
> +
> +         next = next->next;
> +       }
> +
> +
> +      if (next == NULL)
> +       return NULL;
> +
> +
> +      next->next = NULL;
> +      next->line = xstrdup (l->line);
> +      next->control_type = l->control_type;
> +      next->body_count = l->body_count;
> +
> +
> +      if (l->body_count > 0)
> +       {
> +         int i;
> +         struct command_line **blist = l->body_list;
> +
> +         next->body_list =
> +           (struct command_line **) xmalloc (sizeof (struct command_line *)
> +                                             * l->body_count);
> +
> +         for (i = 0; i < l->body_count; i++, blist++)
> +           next->body_list[i] = dup_command_lines (*blist);
> +       }
> +    }
> +
> +  return dup;
> +}
> +
> +
>  /* Free a chain of struct command_line's.  */
> 
>  void
> 
> --
> -Don
> dhoward@redhat.com
> gdb engineering