From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8423 invoked by alias); 2 Sep 2009 08:16:16 -0000 Received: (qmail 8407 invoked by uid 22791); 2 Sep 2009 08:16:11 -0000 X-SWARE-Spam-Status: No, hits=-2.9 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from lo.gmane.org (HELO lo.gmane.org) (80.91.229.12) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 02 Sep 2009 08:16:04 +0000 Received: from list by lo.gmane.org with local (Exim 4.50) id 1Mil0b-0001sN-IS for gdb-patches@sources.redhat.com; Wed, 02 Sep 2009 10:16:01 +0200 Received: from h86-62-88-129.ln.rinet.ru ([86.62.88.129]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Wed, 02 Sep 2009 10:16:01 +0200 Received: from vladimir by h86-62-88-129.ln.rinet.ru with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Wed, 02 Sep 2009 10:16:01 +0200 To: gdb-patches@sources.redhat.com From: Vladimir Prus Subject: Re: GDB MI Reverse Commands added [1 of 3] Date: Wed, 02 Sep 2009 08:16:00 -0000 Message-ID: References: <00ce01ca265a$ccb66ca0$662345e0$@com> <4A95E670.9040402@vmware.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8Bit User-Agent: KNode/0.10.9 X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-09/txt/msg00050.txt.bz2 Michael Snyder wrote: > Index: mi-main.c > =================================================================== > RCS file: /cvs/src/src/gdb/mi/mi-main.c,v > retrieving revision 1.156 > diff -u -p -r1.156 mi-main.c > --- mi-main.c   2 Jul 2009 17:25:59 -0000       1.156 > +++ mi-main.c   27 Aug 2009 01:45:23 -0000 > @@ -88,8 +88,8 @@ static void mi_cmd_execute (struct mi_pa > > static void mi_execute_cli_command (const char *cmd, int args_p, > const char *args); > -static void mi_execute_async_cli_command (char *cli_command, > -                                                       char **argv, int argc); > +static void mi_execute_async_cli_command (char *cli_command, > +                                         char **argv, int argc); > static int register_changed_p (int regnum, struct regcache *, > struct regcache *); > static void get_register (struct frame_info *, int regnum, int format); > @@ -119,35 +119,50 @@ void > mi_cmd_exec_next (char *command, char **argv, int argc) > { > /* FIXME: Should call a libgdb function, not a cli wrapper.  */ > -  mi_execute_async_cli_command ("next", argv, argc); > +  if (argc > 0 && strcmp(argv[0], "--reverse") == 0) > +    mi_execute_async_cli_command ("reverse-next", argv + 1, argc - 1); > +  else > +    mi_execute_async_cli_command ("next", argv, argc); > } > > void > mi_cmd_exec_next_instruction (char *command, char **argv, int argc) > { > /* FIXME: Should call a libgdb function, not a cli wrapper.  */ > -  mi_execute_async_cli_command ("nexti", argv, argc); > +  if (argc > 0 && strcmp(argv[0], "--reverse") == 0) > +    mi_execute_async_cli_command ("reverse-nexti", argv + 1, argc - 1); > +  else > +    mi_execute_async_cli_command ("nexti", argv, argc); > } > > void > mi_cmd_exec_step (char *command, char **argv, int argc) > { > /* FIXME: Should call a libgdb function, not a cli wrapper.  */ > -  mi_execute_async_cli_command ("step", argv, argc); > +  if (argc > 0 && strcmp(argv[0], "--reverse") == 0) > +    mi_execute_async_cli_command ("reverse-step", argv + 1, argc - 1); > +  else > +    mi_execute_async_cli_command ("step", argv, argc); > } > > void > mi_cmd_exec_step_instruction (char *command, char **argv, int argc) > { > /* FIXME: Should call a libgdb function, not a cli wrapper.  */ > -  mi_execute_async_cli_command ("stepi", argv, argc); > +  if (argc > 0 && strcmp(argv[0], "--reverse") == 0) > +    mi_execute_async_cli_command ("reverse-stepi", argv + 1, argc - 1); > +  else > +    mi_execute_async_cli_command ("stepi", argv, argc); > } > > void > mi_cmd_exec_finish (char *command, char **argv, int argc) > { > /* FIXME: Should call a libgdb function, not a cli wrapper.  */ > -  mi_execute_async_cli_command ("finish", argv, argc); > +  if (argc > 0 && strcmp(argv[0], "--reverse") == 0) > +    mi_execute_async_cli_command ("reverse-finish", argv + 1, argc - 1); > +  else > +    mi_execute_async_cli_command ("finish", argv, argc); > } What will happen if MI client does not pass --reverse, but execution_direction is set to reverse via CLI? It seems that this will execute finish in reverse? I think this is not desirable -- rather -exec-finish should always be forward. On the other hand, this is something that I probably can fix as a follow-up patch, somewhere in MI core. > void > @@ -175,7 +190,7 @@ mi_cmd_exec_jump (char *args, char **arg > /* FIXME: Should call a libgdb function, not a cli wrapper.  */ > return mi_execute_async_cli_command ("jump", argv, argc); > } > - > + Stray whitespace change. The patch has a few of those, I imagine that whoever ends up applying the patch to CVS once the copyright assignment is sorted can undo this damage. > static int > proceed_thread_callback (struct thread_info *thread, void *arg) > { > @@ -193,8 +208,8 @@ proceed_thread_callback (struct thread_i > return 0; > } > > -void > -mi_cmd_exec_continue (char *command, char **argv, int argc) > +static void > +exec_continue (char **argv, int argc) > { > if (argc == 0) > continue_1 (0); > @@ -212,10 +227,50 @@ mi_cmd_exec_continue (char *command, cha > > old_chain = make_cleanup_restore_current_thread (); > iterate_over_threads (proceed_thread_callback, &pid); > -      do_cleanups (old_chain); > +      do_cleanups (old_chain); > } > else > -    error ("Usage: -exec-continue [--all|--thread-group id]"); > +    error ("Usage: -exec-continue [--reverse] [--all|--thread-group id]"); > +} > + > +/* continue in reverse direction: > +   XXX: code duplicated from reverse.c */ > + > +static void > +exec_direction_default (void *notused) > +{ > +  /* Return execution direction to default state.  */ > +  execution_direction = EXEC_FORWARD; > +} > + > +static void > +exec_reverse_continue (char **argv, int argc) > +{ > +  enum exec_direction_kind dir = execution_direction; > +  struct cleanup *old_chain; > + > +  if (dir == EXEC_ERROR) > +    error (_("Target %s does not support this command."), target_shortname); > + > +  if (dir == EXEC_REVERSE) > +    error (_("Already in reverse mode.")); > + > +  if (!target_can_execute_reverse) > +    error (_("Target %s does not support this command."), target_shortname); > + > +  old_chain = make_cleanup (exec_direction_default, NULL); > +  execution_direction = EXEC_REVERSE; > +  exec_continue (argv, argc); > +  do_cleanups (old_chain); > +} Why is this code "duplicated from reverse.c"? In other words, cannot mi_cmd_exec_continue call 'reverse_continue'? If this is not possible, there should be a comment explaining why. Otherwise, this is OK. Thanks! - Volodya