RE: [PATCH 1/2] Use mi_getopt_silent

Agovic, Sanimir
2013-08-26T08:58:52Z

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