RE: [PATCH 1/2] Use mi_getopt_silent

["Agovic, Sanimir" <sanimir dot agovic at intel dot com>]

Hello Yao,

I`d rather catch the exception in mi_cmd_stack_list_args to prevent the 
error to bubble up to upper layers. This requires the use of 
throw_error (NOT_FOUND_ERROR, ...) instead of error (...) in mi_getopt
to catch the right exception (*) and re-throw otherwise.
Due to the poor exception handling in gdb this may lead to some boilerplate
code. On the other side it keeps the interface simple & consistent 
e.g. (*) in the patch below mi_getopt_silent may still be verbose/throw.

-Sanimir

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf
> Of Yao Qi
> Sent: Sunday, August 25, 2013 05:42 AM
> To: gdb-patches@sourceware.org
> Subject: [PATCH 1/2] Use mi_getopt_silent
> 
> This patch is to add a new function mi_getopt_silent, which returns -1
> silently (without throwing error) when unknown option is met, and use
> this function to parse options for command '-stack-list-arguments'.
> 
> It makes easier to add a new option in patch 2/2.
> gdb:
> 
> 2013-08-24  Yao Qi  <yao@codesourcery.com>
> 
> 	* mi/mi-cmd-stack.c (parse_no_frames_option): Remove.
> 	(mi_cmd_stack_list_args): Use mi_getopt_silent to handle
> 	options.	
> 	* mi/mi-getopt.c (mi_getopt): Remove.
> 	(mi_getopt_1): Renamed from mi_getopt.  Add one parameter
> 	'error_on_unknown'.
> 	(mi_getopt): Call mi_getopt_1.
> 	(mi_getopt_silent): New.
> 	* mi/mi-getopt.h (mi_getopt_silent): Declare.
> ---
>  gdb/mi/mi-cmd-stack.c |   50 ++++++++++++++++++++++++++++++------------------
>  gdb/mi/mi-getopt.c    |   35 ++++++++++++++++++++++++++++-----
>  gdb/mi/mi-getopt.h    |    8 +++++-
>  3 files changed, 66 insertions(+), 27 deletions(-)
> 
> diff --git a/gdb/mi/mi-cmd-stack.c b/gdb/mi/mi-cmd-stack.c
> index e542fc1..b280e7d 100644
> --- a/gdb/mi/mi-cmd-stack.c
> +++ b/gdb/mi/mi-cmd-stack.c
> @@ -54,17 +54,6 @@ mi_cmd_enable_frame_filters (char *command, char **argv, int argc)
>    frame_filters = 1;
>  }
> 
> -/* Parse the --no-frame-filters option in commands where we cannot use
> -   mi_getopt. */
> -static int
> -parse_no_frames_option (const char *arg)
> -{
> -  if (arg && (strcmp (arg, "--no-frame-filters") == 0))
> -    return 1;
> -
> -  return 0;
> -}
> -
>  /* Print a list of the stack frames.  Args can be none, in which case
>     we want to print the whole backtrace, or a pair of numbers
>     specifying the frame numbers at which to start and stop the
> @@ -284,19 +273,42 @@ mi_cmd_stack_list_args (char *command, char **argv, int argc)
>    enum print_values print_values;
>    struct ui_out *uiout = current_uiout;
>    int raw_arg = 0;
> +  int oind = 0;
>    enum py_bt_status result = PY_BT_ERROR;
> +  enum opt
> +  {
> +    NO_FRAME_FILTERS,
> +  };
> +  static const struct mi_opt opts[] =
> +    {
> +      {"-no-frame-filters", NO_FRAME_FILTERS, 0},
> +      { 0, 0, 0 }
> +    };
> 
> -  if (argc > 0)
> -    raw_arg = parse_no_frames_option (argv[0]);
> +  while (1)
> +    {
> +      char *oarg;
> +      int opt = mi_getopt_silent ("-stack-list-args", argc, argv, opts,
> +				  &oind, &oarg);
> +
> +      if (opt < 0)
> +	break;
> +      switch ((enum opt) opt)
> +	{
> +	case NO_FRAME_FILTERS:
> +	  raw_arg = oind;
> +	  break;
> +	}
> +    }
> 
> -  if (argc < 1 || (argc > 3 && ! raw_arg) || (argc == 2 && ! raw_arg))
> -    error (_("-stack-list-arguments: Usage: " \
> +  if (argc - oind != 1 && argc - oind != 3)
> +    error (_("-stack-list-arguments: Usage: "	\
>  	     "[--no-frame-filters] PRINT_VALUES [FRAME_LOW FRAME_HIGH]"));
> 
> -  if (argc >= 3)
> +  if (argc - oind == 3)
>      {
> -      frame_low = atoi (argv[1 + raw_arg]);
> -      frame_high = atoi (argv[2 + raw_arg]);
> +      frame_low = atoi (argv[1 + oind]);
> +      frame_high = atoi (argv[2 + oind]);
>      }
>    else
>      {
> @@ -306,7 +318,7 @@ mi_cmd_stack_list_args (char *command, char **argv, int argc)
>        frame_high = -1;
>      }
> 
> -  print_values = mi_parse_print_values (argv[raw_arg]);
> +  print_values = mi_parse_print_values (argv[oind]);
> 
>    /* Let's position fi on the frame at which to start the
>       display. Could be the innermost frame if the whole stack needs
> diff --git a/gdb/mi/mi-getopt.c b/gdb/mi/mi-getopt.c
> index a1e2ccc..0255d22 100644
> --- a/gdb/mi/mi-getopt.c
> +++ b/gdb/mi/mi-getopt.c
> @@ -21,11 +21,14 @@
>  #include "mi-getopt.h"
>  #include "gdb_string.h"
> 
> -int
> -mi_getopt (const char *prefix,
> -	   int argc, char **argv,
> -	   const struct mi_opt *opts,
> -	   int *oind, char **oarg)
> +/* See comments about mi_getopt and mi_getopt_silent in mi-getopt.h.
> +   When there is an unknown option, if ERROR_ON_UNKNOWN is true, it
> +   throws an error, otherwise return -1.  */
> +
> +static int
> +mi_getopt_1 (const char *prefix, int argc, char **argv,
> +	     const struct mi_opt *opts, int *oind, char **oarg,
> +	     int error_on_unknown)
>  {
>    char *arg;
>    const struct mi_opt *opt;
> @@ -71,7 +74,27 @@ mi_getopt (const char *prefix,
>  	  return opt->index;
>  	}
>      }
> -  error (_("%s: Unknown option ``%s''"), prefix, arg + 1);
> +
> +  if (error_on_unknown)
> +    error (_("%s: Unknown option ``%s''"), prefix, arg + 1);
> +  else
> +    return -1;
> +}
> +
> +int
> +mi_getopt (const char *prefix,
> +	   int argc, char **argv,
> +	   const struct mi_opt *opts,
> +	   int *oind, char **oarg)
> +{
> +  return mi_getopt_1 (prefix, argc, argv, opts, oind, oarg, 1);
> +}
> +
> +int
> +mi_getopt_silent (const char *prefix, int argc, char **argv,
> +		  const struct mi_opt *opts, int *oind, char **oarg)
> +{
> +  return mi_getopt_1 (prefix, argc, argv, opts, oind, oarg, 0);
>  }
> 
>  int
> diff --git a/gdb/mi/mi-getopt.h b/gdb/mi/mi-getopt.h
> index 9600353..cabe488 100644
> --- a/gdb/mi/mi-getopt.h
> +++ b/gdb/mi/mi-getopt.h
> @@ -46,11 +46,15 @@ struct mi_opt
>     If ARGV[OPTIND] is not an option, -1 is returned and OPTIND updated
>     to specify the non-option argument.  OPTARG is set to NULL.
> 
> -   mi_getopt() calls ``error("%s: Unknown option %c", prefix,
> -   option)'' if an unknown option is encountered.  */
> +   If an unknown option is encountered, mi_getopt() calls
> +   ``error("%s: Unknown option %c", prefix, option)'' and mi_getopt_silent
> +   returns -1.  */
> 
>  extern int mi_getopt (const char *prefix, int argc, char **argv,
>  		      const struct mi_opt *opt, int *optind, char **optarg);
> +extern int mi_getopt_silent (const char *prefix, int argc, char **argv,
> +			     const struct mi_opt *opts, int *oind,
> +			     char **oarg);
> 
>  /* mi_valid_noargs determines if ARGC/ARGV are a valid set of
>     parameters to satisfy an MI function that is not supposed to
> --
> 1.7.7.6

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052