From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id D635F380DF89 for ; Tue, 28 Jun 2022 14:04:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D635F380DF89 Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-277-EKk2lnojObCmzYHKHJLKcA-1; Tue, 28 Jun 2022 10:04:45 -0400 X-MC-Unique: EKk2lnojObCmzYHKHJLKcA-1 Received: by mail-wm1-f70.google.com with SMTP id j19-20020a05600c191300b003a048196712so3818453wmq.4 for ; Tue, 28 Jun 2022 07:04:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version; bh=M8lA2LohzIEaparY3CMp2vWQIY4gq3e48wApQgwuvpg=; b=FCVsqRjcPDzZSw/IatAOt2kjp6Ul34kP3C3RAOjFVubIXcdZnMW5eJyQmxPzJRWK6Z Ve6iSoy+UDGKAMfDZEHu0UrMl0JVCB8m1jEHI4hnU5i2XBtV7JJO3UuslIU9wo4XA1ly G4nUrG7ZQ31aqtHiqdBMkKqmFmhGwoJ1IX689S/aLdw5UTM9eFXiGFKQ3E6jxQBTmxay 7I5kMujlbXg63u65N+2U3cTRm9Uq7EigjU23quZTLD5nP9r1XXaslTZQMVhKZfON/YXf JJbJ+R+mi8y3bjPsigCfpO+Rdzc5fPQOUBm5CxxeXpM7aNlPllcYizH9SulnsxkoPaQf x+bQ== X-Gm-Message-State: AJIora9EebVd8JdJRhXCn0kcK0yu0uTGAO361MqzDsp+TNle8Wxme1nM BpNPZvmS6dQPw1nAEG7bzWZGhcDV1/F0ALPWCzGn3pdlBFDd6S/mrebMYto+OMZLPeG2kw0EyhQ DrtIwCPasNCqBHGCSt+Ga0A== X-Received: by 2002:adf:e9c4:0:b0:21b:8397:860b with SMTP id l4-20020adfe9c4000000b0021b8397860bmr17160623wrn.546.1656425082805; Tue, 28 Jun 2022 07:04:42 -0700 (PDT) X-Google-Smtp-Source: AGRyM1vgQWjaa7gF1weYw02U3W+WINmDel1byVh3YnpJA6H9IVCP1Bujdqx4KuExqFbGPyHxHQMctA== X-Received: by 2002:adf:e9c4:0:b0:21b:8397:860b with SMTP id l4-20020adfe9c4000000b0021b8397860bmr17160532wrn.546.1656425081704; Tue, 28 Jun 2022 07:04:41 -0700 (PDT) Received: from localhost (15.72.115.87.dyn.plus.net. [87.115.72.15]) by smtp.gmail.com with ESMTPSA id id18-20020a05600ca19200b0039c871d3191sm13315448wmb.3.2022.06.28.07.04.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Jun 2022 07:04:41 -0700 (PDT) From: Andrew Burgess To: "Maciej W. Rozycki" , gdb-patches@sourceware.org Cc: Simon Sobisch , Tom Tromey Subject: Re: [PATCH v5 6/8] GDB: Allow arbitrary keywords in integer set commands In-Reply-To: References: Date: Tue, 28 Jun 2022 15:04:39 +0100 Message-ID: <877d50evew.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-2.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_ASCII_DIVIDERS, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Jun 2022 14:04:52 -0000 "Maciej W. Rozycki" writes: > Rather than just `unlimited' allow the integer set commands (or command > options) to define arbitrary keywords for the user to use, removing > hardcoded arrangements for the `unlimited' keyword. > > Make obsolete the confusingly named `var_zinteger', `var_zuinteger' and > `var_zuinteger_unlimited' `set'/`show' command variable types redefining > them in terms of `var_uinteger', `var_integer' and `var_pinteger', which > have the range of [0;UINT_MAX], [INT_MIN;INT_MAX], and [0;INT_MAX] each. > > Following existing practice `var_pinteger' allows extra negative values > to be used, however unlike `var_zuinteger_unlimited' any number of such > values can be defined rather than just `-1'. > > The "p" in `var_pinteger' stands for "positive", for the lack of a more > appropriate unambiguous letter, even though 0 obviously is not positive; > "n" would be confusing as to whether it stands for "non-negative" or > "negative". While looking at this patch I was thinking to myself, I wonder why we need var_pinteger, can't we achieve the same thing using var_uinteger and different literals? So I think I've convinced myself that you don't actually make use of var_pinteger in this patch, and I'm pretty sure you don't use it in any of the later patches either. Maybe I'm wrong, so could you point out where it's used? If it's not used then I think we should remove it. If it's only used in a later commit then I think we should split its addition out into a later commit. If its used in this commit, then I'm sorry for missing it! > > Add a new structure, `literal_def', the entries of which define extra > keywords allowed for a command and numerical values they correspond to. > Those values are not verified against the basic range supported by the > underlying variable type, allowing extra values to be allowed outside > that range, which may or may not be individually made visible to the > user. An optional value translation is possible with the structure to > follow the existing practice for some commands where user-entered 0 is > internally translated to UINT_MAX or INT_MAX. Such translation can now > be arbitrary. Literals defined by this structure are automatically used > for completion as necessary. > > So for example: > > const literal_def integer_unlimited_literals[] = > { > { "unlimited", 0, INT_MAX, true }, > { nullptr } > }; > > defines an extra `unlimited' keyword and a user-visible 0 value, both of > which get translated to INT_MAX for the setting to be used with. > > Similarly: > > const literal_def zuinteger_unlimited_literals[] = > { > { "unlimited", -1, -1, true }, > { nullptr } > }; > > defines the same keyword and a corresponding user-visible -1 value that is > used for the requested setting. If the last member was set to `false' > here, then only the keyword would be allowed for the user to enter and > while -1 would still be used internally trying to enter it as a part of a > command would result in an "integer -1 out of range" error. > > The obsolete command variable types still remain for Guile and Python > use. Eventually they need to be removed, however that would require a > further rework of our Guile and Python support code. Given that these > variable types have been exported as keywords for those languages, those > definitions have to stay, however it would be better to redefine them in > terms of the modern variable types and overall export a better interface > that reflects the flexibility of our core code now. > > There is no user-visible change from this rework, so no documentation or > testsuite updates. > --- > Hi, > > NB I have observed that the value of INT_MIN is not correctly handled: > internally -2147483648 is interpreted as 2147483648 and consequently such > a value is rejected, e.g: > > (gdb) show heuristic-fence-post > The distance searched for the start of a function is 0. > (gdb) set heuristic-fence-post -2147483647 > (gdb) show heuristic-fence-post > The distance searched for the start of a function is -2147483647. > (gdb) set heuristic-fence-post -2147483648 > integer 2147483648 out of range > (gdb) > > umm... (not that it makes sense to set negative values for the heuristic > fence post, but that's irrelevant here). I haven't tried to track the > root cause for this. > > Maciej > > No change from v4. > > New change in v4. > --- > gdb/cli/cli-cmds.c | 56 ++++---- > gdb/cli/cli-decode.c | 302 ++++++++++++++++++++++++++++++++++++++--------- > gdb/cli/cli-option.c | 116 +++++++++++------- > gdb/cli/cli-option.h | 34 +++-- > gdb/cli/cli-setshow.c | 249 ++++++++++++++++++-------------------- > gdb/cli/cli-setshow.h | 20 +-- > gdb/command.h | 108 ++++++++++++++-- > gdb/guile/scm-param.c | 102 +++++++++------ > gdb/maint-test-options.c | 4 > gdb/python/py-param.c | 39 ++++-- > gdb/python/python.c | 49 +++---- > gdb/valprint.c | 9 - > 12 files changed, 716 insertions(+), 372 deletions(-) > > gdb-setshow-cmd-extra-literals.diff > Index: src/gdb/cli/cli-cmds.c > =================================================================== > --- src.orig/gdb/cli/cli-cmds.c > +++ src/gdb/cli/cli-cmds.c > @@ -2200,22 +2200,37 @@ value_from_setting (const setting &var, > { > switch (var.type ()) > { > + case var_uinteger: > case var_integer: > - if (var.get () == INT_MAX) > - return value_from_longest (builtin_type (gdbarch)->builtin_int, > - 0); > - else > - return value_from_longest (builtin_type (gdbarch)->builtin_int, > - var.get ()); > - case var_zinteger: > - return value_from_longest (builtin_type (gdbarch)->builtin_int, > - var.get ()); > + case var_pinteger: > + { > + LONGEST value > + = (var.type () == var_uinteger > + ? static_cast (var.get ()) > + : static_cast (var.get ())); > + > + if (var.extra_literals () != nullptr) > + for (const literal_def *l = var.extra_literals (); > + l->literal != nullptr; > + l++) > + if (value == l->use) > + { > + value = l->val; > + break; > + } > + > + if (var.type () == var_uinteger) > + return > + value_from_ulongest (builtin_type (gdbarch)->builtin_unsigned_int, > + static_cast (value)); > + else > + return > + value_from_longest (builtin_type (gdbarch)->builtin_int, > + static_cast (value)); > + } > case var_boolean: > return value_from_longest (builtin_type (gdbarch)->builtin_int, > var.get () ? 1 : 0); > - case var_zuinteger_unlimited: > - return value_from_longest (builtin_type (gdbarch)->builtin_int, > - var.get ()); > case var_auto_boolean: > { > int val; > @@ -2237,17 +2252,6 @@ value_from_setting (const setting &var, > return value_from_longest (builtin_type (gdbarch)->builtin_int, > val); > } > - case var_uinteger: > - if (var.get () == UINT_MAX) > - return value_from_ulongest > - (builtin_type (gdbarch)->builtin_unsigned_int, 0); > - else > - return value_from_ulongest > - (builtin_type (gdbarch)->builtin_unsigned_int, > - var.get ()); > - case var_zuinteger: > - return value_from_ulongest (builtin_type (gdbarch)->builtin_unsigned_int, > - var.get ()); > case var_string: > case var_string_noescape: > case var_optional_filename: > @@ -2317,13 +2321,11 @@ str_value_from_setting (const setting &v > { > switch (var.type ()) > { > + case var_uinteger: > case var_integer: > - case var_zinteger: > + case var_pinteger: > case var_boolean: > - case var_zuinteger_unlimited: > case var_auto_boolean: > - case var_uinteger: > - case var_zuinteger: > { > std::string cmd_val = get_setshow_command_value_string (var); > > Index: src/gdb/cli/cli-decode.c > =================================================================== > --- src.orig/gdb/cli/cli-decode.c > +++ src/gdb/cli/cli-decode.c > @@ -580,11 +580,51 @@ add_setshow_cmd_full_erased (const char > return {set, show}; > } > > +/* Completes on integer commands that support extra literals. */ > + > +static void > +integer_literals_completer (struct cmd_list_element *c, > + completion_tracker &tracker, > + const char *text, const char *word) > +{ > + const literal_def *extra_literals = c->var->extra_literals (); > + > + if (*text == '\0') > + { > + tracker.add_completion (make_unique_xstrdup ("NUMBER")); > + for (const literal_def *l = extra_literals; > + l->literal != nullptr; > + l++) > + tracker.add_completion (make_unique_xstrdup (l->literal)); > + } > + else > + for (const literal_def *l = extra_literals; > + l->literal != nullptr; > + l++) > + if (startswith (l->literal, text)) > + tracker.add_completion (make_unique_xstrdup (l->literal)); > +} > + > +/* Add element named NAME to both the command SET_LIST and SHOW_LIST. > + THECLASS is as in add_cmd. VAR_TYPE is the kind of thing we are > + setting. VAR is address of the variable being controlled by this > + command. EXTRA_LITERALS if non-NULL define extra literals to be > + accepted in lieu of a number for integer variables. If nullptr is > + given as VAR, then both SET_SETTING_FUNC and GET_SETTING_FUNC must > + be provided. SET_SETTING_FUNC and GET_SETTING_FUNC are callbacks > + used to access and modify the underlying property, whatever its > + storage is. SET_FUNC and SHOW_FUNC are the callback functions > + (if non-NULL). SET_DOC, SHOW_DOC and HELP_DOC are the > + documentation strings. > + > + Return the newly created set and show commands. */ > + > template > static set_show_commands > add_setshow_cmd_full (const char *name, > enum command_class theclass, > var_types var_type, T *var, > + const literal_def *extra_literals, > const char *set_doc, const char *show_doc, > const char *help_doc, > typename setting_func_types::set set_setting_func, > @@ -595,18 +635,44 @@ add_setshow_cmd_full (const char *name, > struct cmd_list_element **show_list) > { > auto erased_args > - = setting::erase_args (var_type, var, > + = setting::erase_args (var_type, var, extra_literals, > set_setting_func, get_setting_func); > + auto cmds = add_setshow_cmd_full_erased (name, > + theclass, > + var_type, erased_args, > + set_doc, show_doc, > + help_doc, > + set_func, > + show_func, > + set_list, > + show_list); > > - return add_setshow_cmd_full_erased (name, > - theclass, > - var_type, erased_args, > - set_doc, show_doc, > - help_doc, > - set_func, > - show_func, > - set_list, > - show_list); > + if (extra_literals != nullptr) > + set_cmd_completer (cmds.set, integer_literals_completer); > + > + return cmds; > +} > + > +/* Same as above but omitting EXTRA_LITERALS. */ > + > +template > +static set_show_commands > +add_setshow_cmd_full (const char *name, > + enum command_class theclass, > + var_types var_type, T *var, > + const char *set_doc, const char *show_doc, > + const char *help_doc, > + typename setting_func_types::set set_setting_func, > + typename setting_func_types::get get_setting_func, > + cmd_func_ftype *set_func, > + show_value_ftype *show_func, > + struct cmd_list_element **set_list, > + struct cmd_list_element **show_list) > +{ > + return add_setshow_cmd_full (name, theclass, var_type, var, nullptr, > + set_doc, show_doc, help_doc, > + set_setting_func, get_setting_func, > + set_func, show_func, set_list, show_list); > } > > /* Add element named NAME to command list LIST (the list for set or > @@ -975,25 +1041,6 @@ add_setshow_optional_filename_cmd (const > return cmds; > } > > -/* Completes on literal "unlimited". Used by integer commands that > - support a special "unlimited" value. */ > - > -static void > -integer_unlimited_completer (struct cmd_list_element *ignore, > - completion_tracker &tracker, > - const char *text, const char *word) > -{ > - static const char * const keywords[] = > - { > - "unlimited", > - NULL, > - }; > - > - if (*text == '\0') > - tracker.add_completion (make_unique_xstrdup ("NUMBER")); > - complete_on_enum (tracker, keywords, text, word); > -} > - > /* Add element named NAME to both the set and show command LISTs (the > list for set/show or some sublist thereof). CLASS is as in > add_cmd. VAR is address of the variable which will contain the > @@ -1002,6 +1049,55 @@ integer_unlimited_completer (struct cmd_ > > set_show_commands > add_setshow_integer_cmd (const char *name, enum command_class theclass, > + int *var, const literal_def *extra_literals, > + const char *set_doc, const char *show_doc, > + const char *help_doc, > + cmd_func_ftype *set_func, > + show_value_ftype *show_func, > + struct cmd_list_element **set_list, > + struct cmd_list_element **show_list) > +{ > + set_show_commands commands > + = add_setshow_cmd_full (name, theclass, var_integer, var, > + extra_literals, set_doc, show_doc, > + help_doc, nullptr, nullptr, set_func, > + show_func, set_list, show_list); > + return commands; > +} > + > +/* Same as above but using a getter and a setter function instead of a pointer > + to a global storage buffer. */ > + > +set_show_commands > +add_setshow_integer_cmd (const char *name, command_class theclass, > + const literal_def *extra_literals, > + const char *set_doc, const char *show_doc, > + const char *help_doc, > + setting_func_types::set set_func, > + setting_func_types::get get_func, > + show_value_ftype *show_func, > + cmd_list_element **set_list, > + cmd_list_element **show_list) > +{ > + auto cmds = add_setshow_cmd_full (name, theclass, var_integer, nullptr, > + extra_literals, set_doc, show_doc, > + help_doc, set_func, get_func, nullptr, > + show_func, set_list, show_list); > + return cmds; > +} > + > +/* Accept `unlimited' or 0, translated internally to INT_MAX. */ > +const literal_def integer_unlimited_literals[] = > + { > + { "unlimited", 0, INT_MAX, true }, > + { nullptr } > + }; > + > +/* Same as above but using `integer_unlimited_literals', with a pointer > + to a global storage buffer. */ > + > +set_show_commands > +add_setshow_integer_cmd (const char *name, enum command_class theclass, > int *var, > const char *set_doc, const char *show_doc, > const char *help_doc, > @@ -1012,12 +1108,10 @@ add_setshow_integer_cmd (const char *nam > { > set_show_commands commands > = add_setshow_cmd_full (name, theclass, var_integer, var, > + integer_unlimited_literals, > set_doc, show_doc, help_doc, > nullptr, nullptr, set_func, > show_func, set_list, show_list); > - > - set_cmd_completer (commands.set, integer_unlimited_completer); > - > return commands; > } > > @@ -1035,12 +1129,54 @@ add_setshow_integer_cmd (const char *nam > cmd_list_element **show_list) > { > auto cmds = add_setshow_cmd_full (name, theclass, var_integer, nullptr, > + integer_unlimited_literals, > set_doc, show_doc, help_doc, set_func, > get_func, nullptr, show_func, set_list, > show_list); > + return cmds; > +} > > - set_cmd_completer (cmds.set, integer_unlimited_completer); > +/* Add element named NAME to both the set and show command LISTs (the > + list for set/show or some sublist thereof). CLASS is as in > + add_cmd. VAR is address of the variable which will contain the > + value. SET_DOC and SHOW_DOC are the documentation strings. */ > + > +set_show_commands > +add_setshow_pinteger_cmd (const char *name, enum command_class theclass, > + int *var, const literal_def *extra_literals, > + const char *set_doc, const char *show_doc, > + const char *help_doc, > + cmd_func_ftype *set_func, > + show_value_ftype *show_func, > + struct cmd_list_element **set_list, > + struct cmd_list_element **show_list) > +{ > + set_show_commands commands > + = add_setshow_cmd_full (name, theclass, var_pinteger, var, > + extra_literals, set_doc, show_doc, > + help_doc, nullptr, nullptr, set_func, > + show_func, set_list, show_list); > + return commands; > +} > > +/* Same as above but using a getter and a setter function instead of a pointer > + to a global storage buffer. */ > + > +set_show_commands > +add_setshow_pinteger_cmd (const char *name, command_class theclass, > + const literal_def *extra_literals, > + const char *set_doc, const char *show_doc, > + const char *help_doc, > + setting_func_types::set set_func, > + setting_func_types::get get_func, > + show_value_ftype *show_func, > + cmd_list_element **set_list, > + cmd_list_element **show_list) > +{ > + auto cmds = add_setshow_cmd_full (name, theclass, var_pinteger, nullptr, > + extra_literals, set_doc, show_doc, > + help_doc, set_func, get_func, nullptr, > + show_func, set_list, show_list); > return cmds; > } > > @@ -1051,7 +1187,7 @@ add_setshow_integer_cmd (const char *nam > > set_show_commands > add_setshow_uinteger_cmd (const char *name, enum command_class theclass, > - unsigned int *var, > + unsigned int *var, const literal_def *extra_literals, > const char *set_doc, const char *show_doc, > const char *help_doc, > cmd_func_ftype *set_func, > @@ -1061,12 +1197,9 @@ add_setshow_uinteger_cmd (const char *na > { > set_show_commands commands > = add_setshow_cmd_full (name, theclass, var_uinteger, var, > - set_doc, show_doc, help_doc, > - nullptr, nullptr, set_func, > + extra_literals, set_doc, show_doc, > + help_doc, nullptr, nullptr, set_func, > show_func, set_list, show_list); > - > - set_cmd_completer (commands.set, integer_unlimited_completer); > - > return commands; > } > > @@ -1075,6 +1208,7 @@ add_setshow_uinteger_cmd (const char *na > > set_show_commands > add_setshow_uinteger_cmd (const char *name, command_class theclass, > + const literal_def *extra_literals, > const char *set_doc, const char *show_doc, > const char *help_doc, > setting_func_types::set set_func, > @@ -1084,13 +1218,63 @@ add_setshow_uinteger_cmd (const char *na > cmd_list_element **show_list) > { > auto cmds = add_setshow_cmd_full (name, theclass, var_uinteger, > - nullptr, set_doc, show_doc, > - help_doc, set_func, get_func, > - nullptr, show_func, set_list, > + nullptr, extra_literals, > + set_doc, show_doc, help_doc, > + set_func, get_func, nullptr, > + show_func, set_list, > show_list); > + return cmds; > +} > > - set_cmd_completer (cmds.set, integer_unlimited_completer); > +/* Accept `unlimited' or 0, translated internally to UINT_MAX. */ > +const literal_def uinteger_unlimited_literals[] = > + { > + { "unlimited", 0, UINT_MAX, true }, > + { nullptr } > + }; > + > +/* Same as above but using `uinteger_unlimited_literals', with a pointer > + to a global storage buffer. */ > + > +set_show_commands > +add_setshow_uinteger_cmd (const char *name, enum command_class theclass, > + unsigned int *var, > + const char *set_doc, const char *show_doc, > + const char *help_doc, > + cmd_func_ftype *set_func, > + show_value_ftype *show_func, > + struct cmd_list_element **set_list, > + struct cmd_list_element **show_list) > +{ > + set_show_commands commands > + = add_setshow_cmd_full (name, theclass, var_uinteger, var, > + uinteger_unlimited_literals, > + set_doc, show_doc, help_doc, nullptr, > + nullptr, set_func, show_func, > + set_list, show_list); > + return commands; > +} > + > +/* Same as above but using a getter and a setter function instead of a pointer > + to a global storage buffer. */ > > +set_show_commands > +add_setshow_uinteger_cmd (const char *name, command_class theclass, > + const char *set_doc, const char *show_doc, > + const char *help_doc, > + setting_func_types::set set_func, > + setting_func_types::get get_func, > + show_value_ftype *show_func, > + cmd_list_element **set_list, > + cmd_list_element **show_list) > +{ > + auto cmds = add_setshow_cmd_full (name, theclass, var_uinteger, > + nullptr, > + uinteger_unlimited_literals, > + set_doc, show_doc, help_doc, > + set_func, get_func, nullptr, > + show_func, set_list, > + show_list); > return cmds; > } > > @@ -1109,7 +1293,7 @@ add_setshow_zinteger_cmd (const char *na > struct cmd_list_element **set_list, > struct cmd_list_element **show_list) > { > - return add_setshow_cmd_full (name, theclass, var_zinteger, var, > + return add_setshow_cmd_full (name, theclass, var_integer, var, > set_doc, show_doc, help_doc, > nullptr, nullptr, set_func, > show_func, set_list, show_list); > @@ -1128,12 +1312,22 @@ add_setshow_zinteger_cmd (const char *na > cmd_list_element **set_list, > cmd_list_element **show_list) > { > - return add_setshow_cmd_full (name, theclass, var_zinteger, nullptr, > + return add_setshow_cmd_full (name, theclass, var_integer, nullptr, > set_doc, show_doc, help_doc, set_func, > get_func, nullptr, show_func, set_list, > show_list); > } > > +/* Accept `unlimited' or -1, using -1 internally. */ > +const literal_def zuinteger_unlimited_literals[] = > + { > + { "unlimited", -1, -1, true }, > + { nullptr } > + }; > + > +/* Same as above but using `zuinteger_unlimited_literals', with a pointer > + to a global storage buffer. */ > + > set_show_commands > add_setshow_zuinteger_unlimited_cmd (const char *name, > enum command_class theclass, > @@ -1147,13 +1341,11 @@ add_setshow_zuinteger_unlimited_cmd (con > struct cmd_list_element **show_list) > { > set_show_commands commands > - = add_setshow_cmd_full (name, theclass, var_zuinteger_unlimited, var, > + = add_setshow_cmd_full (name, theclass, var_pinteger, var, > + zuinteger_unlimited_literals, > set_doc, show_doc, help_doc, nullptr, > nullptr, set_func, show_func, set_list, > show_list); > - > - set_cmd_completer (commands.set, integer_unlimited_completer); > - > return commands; > } > > @@ -1171,13 +1363,11 @@ add_setshow_zuinteger_unlimited_cmd (con > cmd_list_element **show_list) > { > auto cmds > - = add_setshow_cmd_full (name, theclass, var_zuinteger_unlimited, > - nullptr, set_doc, show_doc, help_doc, set_func, > + = add_setshow_cmd_full (name, theclass, var_pinteger, nullptr, > + zuinteger_unlimited_literals, > + set_doc, show_doc, help_doc, set_func, > get_func, nullptr, show_func, set_list, > show_list); > - > - set_cmd_completer (cmds.set, integer_unlimited_completer); > - > return cmds; > } > > @@ -1196,7 +1386,7 @@ add_setshow_zuinteger_cmd (const char *n > struct cmd_list_element **set_list, > struct cmd_list_element **show_list) > { > - return add_setshow_cmd_full (name, theclass, var_zuinteger, > + return add_setshow_cmd_full (name, theclass, var_uinteger, > var, set_doc, show_doc, help_doc, > nullptr, nullptr, set_func, > show_func, set_list, show_list); > @@ -1215,7 +1405,7 @@ add_setshow_zuinteger_cmd (const char *n > cmd_list_element **set_list, > cmd_list_element **show_list) > { > - return add_setshow_cmd_full (name, theclass, var_zuinteger, > + return add_setshow_cmd_full (name, theclass, var_uinteger, > nullptr, set_doc, show_doc, > help_doc, set_func, get_func, > nullptr, show_func, set_list, > Index: src/gdb/cli/cli-option.c > =================================================================== > --- src.orig/gdb/cli/cli-option.c > +++ src/gdb/cli/cli-option.c > @@ -38,7 +38,7 @@ union option_value > /* For var_uinteger options. */ > unsigned int uinteger; > > - /* For var_zuinteger_unlimited options. */ > + /* For var_integer and var_pinteger options. */ > int integer; > > /* For var_enum options. */ > @@ -356,42 +356,55 @@ parse_option (gdb::array_view return option_def_and_value {*match, match_ctx, val}; > } > case var_uinteger: > - case var_zuinteger_unlimited: > + case var_integer: > + case var_pinteger: > { > if (completion != nullptr) > { > - if (**args == '\0') > - { > - /* Convenience to let the user know what the option > - can accept. Note there's no common prefix between > - the strings on purpose, so that readline doesn't do > - a partial match. */ > - completion->tracker.add_completion > - (make_unique_xstrdup ("NUMBER")); > - completion->tracker.add_completion > - (make_unique_xstrdup ("unlimited")); > - return {}; > - } > - else if (startswith ("unlimited", *args)) > + if (match->extra_literals != nullptr) > { > - completion->tracker.add_completion > - (make_unique_xstrdup ("unlimited")); > - return {}; > + /* Convenience to let the user know what the option can > + accept. Make sure there's no common prefix between > + "NUMBER" and all the strings when adding new ones, > + so that readline doesn't do a partial match. */ > + if (**args == '\0') > + { > + completion->tracker.add_completion > + (make_unique_xstrdup ("NUMBER")); > + for (const literal_def *l = match->extra_literals; > + l->literal != nullptr; > + l++) > + completion->tracker.add_completion > + (make_unique_xstrdup (l->literal)); > + return {}; > + } > + else > + { > + bool completions = false; > + for (const literal_def *l = match->extra_literals; > + l->literal != nullptr; > + l++) > + if (startswith (l->literal, *args)) > + { > + completion->tracker.add_completion > + (make_unique_xstrdup (l->literal)); > + completions = true; > + } > + if (completions) > + return {}; > + } > } > } > > - if (match->type == var_zuinteger_unlimited) > - { > - option_value val; > - val.integer = parse_cli_var_zuinteger_unlimited (args, false); > - return option_def_and_value {*match, match_ctx, val}; > - } > + LONGEST v = parse_cli_var_integer (match->type, > + match->extra_literals, > + args, false); > + option_value val; > + if (match->type == var_uinteger) > + val.uinteger = v; > else > - { > - option_value val; > - val.uinteger = parse_cli_var_uinteger (match->type, args, false); > - return option_def_and_value {*match, match_ctx, val}; > - } > + val.integer = v; > + return option_def_and_value {*match, match_ctx, val}; > } > case var_enum: > { > @@ -593,7 +606,7 @@ save_option_value_in_ctx (gdb::optional< > *ov->option.var_address.uinteger (ov->option, ov->ctx) > = ov->value->uinteger; > break; > - case var_zuinteger_unlimited: > + case var_integer: > *ov->option.var_address.integer (ov->option, ov->ctx) > = ov->value->integer; > break; > @@ -664,8 +677,20 @@ get_val_type_str (const option_def &opt, > case var_boolean: > return "[on|off]"; > case var_uinteger: > - case var_zuinteger_unlimited: > - return "NUMBER|unlimited"; > + case var_integer: > + case var_pinteger: > + { > + buffer = "NUMBER"; > + if (opt.extra_literals != nullptr) > + for (const literal_def *l = opt.extra_literals; > + l->literal != nullptr; > + l++) > + { > + buffer += '|'; > + buffer += l->literal; > + } > + return buffer.c_str (); > + } > case var_enum: > { > buffer = ""; > @@ -789,20 +814,31 @@ add_setshow_cmds_for_options (command_cl > { > add_setshow_uinteger_cmd (option.name, cmd_class, > option.var_address.uinteger (option, data), > + option.extra_literals, > option.set_doc, option.show_doc, > option.help_doc, > nullptr, option.show_cmd_cb, > set_list, show_list); > } > - else if (option.type == var_zuinteger_unlimited) > + else if (option.type == var_integer) > { > - add_setshow_zuinteger_unlimited_cmd > - (option.name, cmd_class, > - option.var_address.integer (option, data), > - option.set_doc, option.show_doc, > - option.help_doc, > - nullptr, option.show_cmd_cb, > - set_list, show_list); > + add_setshow_integer_cmd (option.name, cmd_class, > + option.var_address.integer (option, data), > + option.extra_literals, > + option.set_doc, option.show_doc, > + option.help_doc, > + nullptr, option.show_cmd_cb, > + set_list, show_list); > + } > + else if (option.type == var_pinteger) > + { > + add_setshow_pinteger_cmd (option.name, cmd_class, > + option.var_address.integer (option, data), > + option.extra_literals, > + option.set_doc, option.show_doc, > + option.help_doc, > + nullptr, option.show_cmd_cb, > + set_list, show_list); > } > else if (option.type == var_enum) > { > Index: src/gdb/cli/cli-option.h > =================================================================== > --- src.orig/gdb/cli/cli-option.h > +++ src/gdb/cli/cli-option.h > @@ -49,12 +49,13 @@ struct option_def > used to create the option's "set/show" commands. */ > constexpr option_def (const char *name_, > var_types var_type_, > + const literal_def *extra_literals_, > erased_get_var_address_ftype *erased_get_var_address_, > show_value_ftype *show_cmd_cb_, > const char *set_doc_, > const char *show_doc_, > const char *help_doc_) > - : name (name_), type (var_type_), > + : name (name_), type (var_type_), extra_literals (extra_literals_), > erased_get_var_address (erased_get_var_address_), > var_address {}, > show_cmd_cb (show_cmd_cb_), > @@ -68,6 +69,9 @@ struct option_def > /* The option's type. */ > var_types type; > > + /* Extra literals, such as `unlimited', accepted in lieu of a number. */ > + const literal_def *extra_literals; > + > /* A function that gets the controlling variable's address, type > erased. */ > erased_get_var_address_ftype *erased_get_var_address; > @@ -160,7 +164,7 @@ struct boolean_option_def : option_def > const char *set_doc_, > const char *show_doc_ = nullptr, > const char *help_doc_ = nullptr) > - : option_def (long_option_, var_boolean, > + : option_def (long_option_, var_boolean, nullptr, > (erased_get_var_address_ftype *) get_var_address_cb_, > show_cmd_cb_, > set_doc_, show_doc_, help_doc_) > @@ -207,11 +211,12 @@ struct uinteger_option_def : option_def > { > uinteger_option_def (const char *long_option_, > unsigned int *(*get_var_address_cb_) (Context *), > + const literal_def *extra_literals_, > show_value_ftype *show_cmd_cb_, > const char *set_doc_, > const char *show_doc_ = nullptr, > const char *help_doc_ = nullptr) > - : option_def (long_option_, var_uinteger, > + : option_def (long_option_, var_uinteger, extra_literals_, > (erased_get_var_address_ftype *) get_var_address_cb_, > show_cmd_cb_, > set_doc_, show_doc_, help_doc_) I think we should add a second constructor for uinteger_option_def like this: uinteger_option_def (const char *long_option_, unsigned int *(*get_var_address_cb_) (Context *), show_value_ftype *show_cmd_cb_, const char *set_doc_, const char *show_doc_ = nullptr, const char *help_doc_ = nullptr) : uinteger_option_def (long_option_, get_var_address_cb_, uinteger_unlimited_literals, show_cmd_cb_, set_doc_, show_doc_, help_doc_) { /* Nothing. */ } This would keep the existing '0 == unlimited' as the default behaviour, but still give the option of overriding the literals if needed. > @@ -220,18 +225,19 @@ struct uinteger_option_def : option_def > } > }; > > -/* A var_zuinteger_unlimited command line option. */ > +/* A var_integer command line option. */ > > template > -struct zuinteger_unlimited_option_def : option_def If instead of deleting zuinteger_unlimited_option_def we redefined the class like this: /* A var_zuinteger_unlimited command line option. */ template struct zuinteger_unlimited_option_def : integer_option_def { zuinteger_unlimited_option_def (const char *long_option_, int *(*get_var_address_cb_) (Context *), show_value_ftype *show_cmd_cb_, const char *set_doc_, const char *show_doc_ = nullptr, const char *help_doc_ = nullptr) : integer_option_def (long_option_, get_var_address_cb_, zuinteger_unlimited_literals, show_cmd_cb_, set_doc_, show_doc_, help_doc_) { /* Nothing. */ } }; We could then still make use of zuinteger_unlimited_option_def if we wanted, which would be an integer with zuinteger_unlimited_literals. With this change, and the previous change I suggest, all the changes in valprint.c could be reverted. > +struct integer_option_def : option_def > { > - zuinteger_unlimited_option_def (const char *long_option_, > - int *(*get_var_address_cb_) (Context *), > - show_value_ftype *show_cmd_cb_, > - const char *set_doc_, > - const char *show_doc_ = nullptr, > - const char *help_doc_ = nullptr) > - : option_def (long_option_, var_zuinteger_unlimited, > + integer_option_def (const char *long_option_, > + int *(*get_var_address_cb_) (Context *), > + const literal_def *extra_literals_, > + show_value_ftype *show_cmd_cb_, > + const char *set_doc_, > + const char *show_doc_ = nullptr, > + const char *help_doc_ = nullptr) > + : option_def (long_option_, var_integer, extra_literals_, > (erased_get_var_address_ftype *) get_var_address_cb_, > show_cmd_cb_, > set_doc_, show_doc_, help_doc_) > @@ -252,7 +258,7 @@ struct enum_option_def : option_def > const char *set_doc_, > const char *show_doc_ = nullptr, > const char *help_doc_ = nullptr) > - : option_def (long_option_, var_enum, > + : option_def (long_option_, var_enum, nullptr, > (erased_get_var_address_ftype *) get_var_address_cb_, > show_cmd_cb_, > set_doc_, show_doc_, help_doc_) > @@ -273,7 +279,7 @@ struct string_option_def : option_def > const char *set_doc_, > const char *show_doc_ = nullptr, > const char *help_doc_ = nullptr) > - : option_def (long_option_, var_string, > + : option_def (long_option_, var_string, nullptr, > (erased_get_var_address_ftype *) get_var_address_cb_, > show_cmd_cb_, > set_doc_, show_doc_, help_doc_) > Index: src/gdb/cli/cli-setshow.c > =================================================================== > --- src.orig/gdb/cli/cli-setshow.c > +++ src/gdb/cli/cli-setshow.c > @@ -149,10 +149,11 @@ deprecated_show_value_hack (struct ui_fi > } > } > > -/* Returns true if ARG is "unlimited". */ > +/* Returns true and the value in VAL if ARG is an accepted literal. */ > > static bool > -is_unlimited_literal (const char **arg, bool expression) > +get_literal_val (LONGEST &val, const literal_def *extra_literals, > + const char **arg, bool expression) > { > *arg = skip_spaces (*arg); > > @@ -162,85 +163,108 @@ is_unlimited_literal (const char **arg, > > size_t len = p - *arg; > > - if (len > 0 && strncmp ("unlimited", *arg, len) == 0) > - { > - *arg += len; > - > - /* If parsing an expression (i.e., parsing for a "set" command), > - anything after "unlimited" is junk. For options, anything > - after "unlimited" might be a command argument or another > - option. */ > - if (expression) > + if (len > 0 && extra_literals != nullptr) > + for (const literal_def *l = extra_literals; > + l->literal != nullptr; > + l++) > + if (strncmp (l->literal, *arg, len) == 0) > { > - const char *after = skip_spaces (*arg); > - if (*after != '\0') > - error (_("Junk after \"%.*s\": %s"), > - (int) len, unl_start, after); > - } > + *arg += len; > > - return true; > - } > + /* If parsing an expression (i.e., parsing for a "set" command), > + anything after the literal is junk. For options, anything > + after the literal might be a command argument or another > + option. */ > + if (expression) > + { > + const char *after = skip_spaces (*arg); > + if (*after != '\0') > + error (_("Junk after \"%.*s\": %s"), > + (int) len, unl_start, after); > + } > + > + val = l->use; > + return true; > + } > > return false; > } > > /* See cli-setshow.h. */ > > -unsigned int > -parse_cli_var_uinteger (var_types var_type, const char **arg, > - bool expression) > +LONGEST > +parse_cli_var_integer (var_types var_type, const literal_def *extra_literals, > + const char **arg, bool expression) > { > LONGEST val; > > if (*arg == nullptr || **arg == '\0') > { > - if (var_type == var_uinteger) > - error_no_arg (_("integer to set it to, or \"unlimited\"")); > - else > + if (extra_literals == nullptr) > error_no_arg (_("integer to set it to")); > - } > - > - if (var_type == var_uinteger && is_unlimited_literal (arg, expression)) > - val = 0; > - else if (expression) > - val = parse_and_eval_long (*arg); > - else > - val = get_ulongest (arg); > - > - if (var_type == var_uinteger && val == 0) > - val = UINT_MAX; > - else if (val < 0 > - /* For var_uinteger, don't let the user set the value > - to UINT_MAX directly, as that exposes an > - implementation detail to the user interface. */ > - || (var_type == var_uinteger && val >= UINT_MAX) > - || (var_type == var_zuinteger && val > UINT_MAX)) > - error (_("integer %s out of range"), plongest (val)); > - > - return val; > -} > - > -/* See cli-setshow.h. */ > + else > + { > + std::string buffer = ""; > + size_t count = 0; > > -int > -parse_cli_var_zuinteger_unlimited (const char **arg, bool expression) > -{ > - LONGEST val; > + for (const literal_def *l = extra_literals; > + l->literal != nullptr; > + l++, count++) > + { > + if (count != 0) > + buffer += ", "; > + buffer = buffer + '"' + l->literal + '"'; > + } > + if (count > 1) > + error_no_arg > + (string_printf (_("integer to set it to, or one of: %s"), > + buffer.c_str ()).c_str ()); > + else > + error_no_arg > + (string_printf (_("integer to set it to, or %s"), > + buffer.c_str ()).c_str ()); > + } > + } > > - if (*arg == nullptr || **arg == '\0') > - error_no_arg (_("integer to set it to, or \"unlimited\"")); > + if (!get_literal_val (val, extra_literals, arg, expression)) > + { > + if (expression) > + val = parse_and_eval_long (*arg); > + else > + val = get_ulongest (arg); > > - if (is_unlimited_literal (arg, expression)) > - val = -1; > - else if (expression) > - val = parse_and_eval_long (*arg); > - else > - val = get_ulongest (arg); > + enum tribool allowed = TRIBOOL_UNKNOWN; > + if (extra_literals != nullptr) > + { > + for (const literal_def *l = extra_literals; > + l->literal != nullptr; > + l++) > + if (val == l->val) > + { > + allowed = l->allow ? TRIBOOL_TRUE : TRIBOOL_FALSE; > + val = l->use; > + break; > + } > + else if (val == l->use) > + { > + allowed = TRIBOOL_FALSE; > + break; > + } > + } I haven't run the testsuite on this, so maybe there's issues, but I'd like to propose that this loop be rewritten as: if (extra_literals != nullptr) { for (const literal_def *l = extra_literals; l->literal != nullptr; l++) if (val == l->val && l->allow) { allowed = TRIBOOL_TRUE; val = l->use; break; } else if (val == l->use) allowed = TRIBOOL_FALSE; } The 'if (val == l->val && l->allow)' would be improved if l->val was changed to an optional. The change to the 'val == l->use' block allows I think another improvement, this: static const literal_def print_characters_literals[] = { { "elements", 0, 0, false }, { "unlimited", 0, UINT_MAX, true }, { nullptr } }; The change here is that: set print characters 0 now behaves the same as: set print elements 0 In both cases, this sets the value to "unlimited". I like the consistency. > > - if (val > INT_MAX) > - error (_("integer %s out of range"), plongest (val)); > - else if (val < -1) > - error (_("only -1 is allowed to set as unlimited")); > + if (allowed == TRIBOOL_UNKNOWN) > + { > + if (var_type == var_pinteger && val < 0) > + error (_("only -1 is allowed to set as unlimited")); > + else if (val > UINT_MAX || val < INT_MIN > + || (var_type == var_uinteger && val < 0) > + || (var_type == var_integer && val > INT_MAX) > + || (var_type == var_pinteger && val > INT_MAX)) > + allowed = TRIBOOL_FALSE; > + } > + if (allowed == TRIBOOL_FALSE) > + error (_("integer %s out of range"), plongest (val)); > + } > > return val; > } > @@ -405,41 +429,18 @@ do_set_command (const char *arg, int fro > option_changed = c->var->set (parse_auto_binary_operation (arg)); > break; > case var_uinteger: > - case var_zuinteger: > option_changed > - = c->var->set (parse_cli_var_uinteger (c->var->type (), > - &arg, true)); > + = c->var->set (parse_cli_var_integer (c->var->type (), > + c->var-> > + extra_literals (), > + &arg, true)); > break; > case var_integer: > - case var_zinteger: > - { > - LONGEST val; > - > - if (*arg == '\0') > - { > - if (c->var->type () == var_integer) > - error_no_arg (_("integer to set it to, or \"unlimited\"")); > - else > - error_no_arg (_("integer to set it to")); > - } > - > - if (c->var->type () == var_integer && is_unlimited_literal (&arg, true)) > - val = 0; > - else > - val = parse_and_eval_long (arg); > - > - if (val == 0 && c->var->type () == var_integer) > - val = INT_MAX; > - else if (val < INT_MIN > - /* For var_integer, don't let the user set the value > - to INT_MAX directly, as that exposes an > - implementation detail to the user interface. */ > - || (c->var->type () == var_integer && val >= INT_MAX) > - || (c->var->type () == var_zinteger && val > INT_MAX)) > - error (_("integer %s out of range"), plongest (val)); > - > - option_changed = c->var->set (val); > - } > + case var_pinteger: > + option_changed > + = c->var->set (parse_cli_var_integer (c->var->type (), > + c->var->extra_literals (), > + &arg, true)); > break; > case var_enum: > { > @@ -454,10 +455,6 @@ do_set_command (const char *arg, int fro > option_changed = c->var->set (match); > } > break; > - case var_zuinteger_unlimited: > - option_changed = c->var->set > - (parse_cli_var_zuinteger_unlimited (&arg, true)); > - break; > default: > error (_("gdb internal error: bad var_type in do_setshow_command")); > } > @@ -551,7 +548,6 @@ do_set_command (const char *arg, int fro > } > break; > case var_uinteger: > - case var_zuinteger: > { > char s[64]; > > @@ -560,8 +556,7 @@ do_set_command (const char *arg, int fro > } > break; > case var_integer: > - case var_zinteger: > - case var_zuinteger_unlimited: > + case var_pinteger: > { > char s[64]; > > @@ -623,36 +618,32 @@ get_setshow_command_value_string (const > } > break; > case var_uinteger: > - case var_zuinteger: > - { > - const unsigned int value = var.get (); > - > - if (var.type () == var_uinteger > - && value == UINT_MAX) > - stb.puts ("unlimited"); > - else > - stb.printf ("%u", value); > - } > - break; > case var_integer: > - case var_zinteger: > + case var_pinteger: > { > - const int value = var.get (); > + bool printed = false; > + const LONGEST value > + = (var.type () == var_uinteger > + ? static_cast (var.get ()) > + : static_cast (var.get ())); > > - if (var.type () == var_integer > - && value == INT_MAX) > - stb.puts ("unlimited"); > - else > - stb.printf ("%d", value); > - } > - break; > - case var_zuinteger_unlimited: > - { > - const int value = var.get (); > - if (value == -1) > - stb.puts ("unlimited"); > - else > - stb.printf ("%d", value); > + if (var.extra_literals () != nullptr) > + for (const literal_def *l = var.extra_literals (); > + l->literal != nullptr; > + l++) > + if (value == l->use) > + { > + stb.puts (l->literal); > + printed = true; > + break; > + } > + if (!printed) > + { > + if (var.type () == var_uinteger) > + stb.printf ("%u", static_cast (value)); > + else > + stb.printf ("%d", static_cast (value)); > + } > } > break; > default: > Index: src/gdb/cli/cli-setshow.h > =================================================================== > --- src.orig/gdb/cli/cli-setshow.h > +++ src/gdb/cli/cli-setshow.h > @@ -29,21 +29,19 @@ extern int parse_cli_boolean_value (cons > past a successfully parsed value. */ > extern int parse_cli_boolean_value (const char **arg); > > -/* Parse ARG, an option to a var_uinteger or var_zuinteger variable. > - Either returns the parsed value on success or throws an error. If > - EXPRESSION is true, *ARG is parsed as an expression; otherwise, it > - is parsed with get_ulongest. It's not possible to parse the > +/* Parse ARG, an option to a var_uinteger, var_integer or var_pinteger > + variable. Return the parsed value on success or throw an error. If > + EXTRA_LITERALS is non-null, then interpret those literals accordingly. > + If EXPRESSION is true, *ARG is parsed as an expression; otherwise, > + it is parsed with get_ulongest. It's not possible to parse the > integer as an expression when there may be valid input after the > integer, such as when parsing command options. E.g., "print > -elements NUMBER -obj --". In such case, parsing as an expression > would parse "-obj --" as part of the expression as well. */ > -extern unsigned int parse_cli_var_uinteger (var_types var_type, > - const char **arg, > - bool expression); > - > -/* Like parse_cli_var_uinteger, for var_zuinteger_unlimited. */ > -extern int parse_cli_var_zuinteger_unlimited (const char **arg, > - bool expression); > +extern LONGEST parse_cli_var_integer (var_types var_type, > + const literal_def *extra_literals, > + const char **arg, > + bool expression); > > /* Parse ARG, an option to a var_enum variable. ENUM is a > null-terminated array of possible values. Either returns the parsed > Index: src/gdb/command.h > =================================================================== > --- src.orig/gdb/command.h > +++ src/gdb/command.h > @@ -84,16 +84,18 @@ typedef enum var_types > value. */ > var_auto_boolean, > > - /* Unsigned Integer. *VAR is an unsigned int. The user can type > - 0 to mean "unlimited", which is stored in *VAR as UINT_MAX. */ > + /* Unsigned Integer. *VAR is an unsigned int. In the Guile and Python > + APIs 0 means unlimited, which is stored in *VAR as UINT_MAX. */ > var_uinteger, > > - /* Like var_uinteger but signed. *VAR is an int. The user can > - type 0 to mean "unlimited", which is stored in *VAR as > - INT_MAX. The only remaining use of it is the Python API. > - Don't use it elsewhere. */ > + /* Like var_uinteger but signed. *VAR is an int. In the Guile and > + Python APIs 0 means unlimited, which is stored in *VAR as INT_MAX. */ > var_integer, > > + /* Like var_integer but negative numbers are not allowed, > + except for special values. *VAR is an int. */ > + var_pinteger, > + > /* String which the user enters with escapes (e.g. the user types > \n and it is a real newline in the stored string). > *VAR is a std::string, "" if the string is empty. */ > @@ -106,15 +108,16 @@ typedef enum var_types > var_optional_filename, > /* String which stores a filename. (*VAR) is a std::string. */ > var_filename, > - /* ZeroableInteger. *VAR is an int. Like var_integer except > - that zero really means zero. */ > + /* Like var_integer. *VAR is an int. The only remaining uses > + of it are the Guile and Python APIs. Don't use it elsewhere. */ > var_zinteger, > - /* ZeroableUnsignedInteger. *VAR is an unsigned int. Zero really > - means zero. */ > + /* Like var_uinteger. *VAR is an unsigned int. The only remaining > + uses of it are the Guile and Python APIs. Don't use it elsewhere. */ > var_zuinteger, > - /* ZeroableUnsignedInteger with unlimited value. *VAR is an int, > - but its range is [0, INT_MAX]. -1 stands for unlimited and > - other negative numbers are not allowed. */ > + /* Like var_pinteger. *VAR is an int, but its range is [-1, INT_MAX]. > + -1 stands for unlimited and other negative numbers are not allowed. > + The only remaining uses of it are the Guile and Python APIs. Don't > + use it elsewhere. */ > var_zuinteger_unlimited, > /* Enumerated type. Can only have one of the specified values. > *VAR is a char pointer to the name of the element that we > @@ -123,6 +126,22 @@ typedef enum var_types > } > var_types; > > +/* A structure describing an extra literal accepted and shown in place > + of a number. */ > +typedef struct literal_defs You don't need the typedef trick as this is C++. > + { > + /* The literal to define, e.g. "unlimited". */ > + const char *literal; > + /* A number optionally accepted that stands for the literal. */ > + LONGEST val; > + /* The number to substitute internally for LITERAL or VAL; > + the use of this number is not allowed (unless the same as VAL). */ > + LONGEST use; > + /* True if the use of VAL is allowed; otherwise only the literal is. */ > + bool allow; Could we replace VAL and ALLOW with something like: gdb::optional .... > + } > +literal_def; > + > /* Return true if a setting of type VAR_TYPE is backed with type T. > > This function is left without definition intentionally. This template is > @@ -153,15 +172,14 @@ inline bool var_type_uses template<> > inline bool var_type_uses (var_types t) > { > - return (t == var_uinteger || t == var_zinteger || t == var_zuinteger); > + return t == var_uinteger; > } Should we really remove all these uses of var_zinteger, etc? The enum values are still around, right? So surely if you passed in that enum field the true/false response shouldn't change... Thanks, Andrew > > /* Return true if a setting of type T is backed by an int variable. */ > template<> > inline bool var_type_uses (var_types t) > { > - return (t == var_integer || t == var_zinteger > - || t == var_zuinteger_unlimited); > + return t == var_integer || t == var_pinteger; > } > > /* Return true if a setting of type T is backed by a std::string variable. */ > @@ -233,6 +251,7 @@ struct setting > struct erased_args > { > void *var; > + const void *extra_literals; > erased_func setter; > erased_func getter; > }; > @@ -240,6 +259,7 @@ struct setting > template > static erased_args erase_args (var_types var_type, > T *var, > + const literal_def *extra_literals, > typename setting_func_types::set set_setting_func, > typename setting_func_types::get get_setting_func) > { > @@ -254,6 +274,7 @@ struct setting > > return { > var, > + extra_literals, > reinterpret_cast (set_setting_func), > reinterpret_cast (get_setting_func) > }; > @@ -265,6 +286,7 @@ struct setting > setting (var_types var_type, const erased_args &args) > : m_var_type (var_type), > m_var (args.var), > + m_extra_literals (args.extra_literals), > m_getter (args.getter), > m_setter (args.setter) > { > @@ -295,6 +317,10 @@ struct setting > var_types type () const > { return m_var_type; } > > + /* Access any extra literals accepted. */ > + const literal_def *extra_literals () const > + { return static_cast (m_extra_literals); } > + > /* Return the current value. > > The template parameter T is the type of the variable used to store the > @@ -357,6 +383,9 @@ struct setting > non-nullptr. */ > void *m_var = nullptr; > > + /* Any extra literals accepted. */ > + const void *m_extra_literals = nullptr; > + > /* Pointer to a user provided getter. */ > erased_func m_getter = nullptr; > > @@ -652,6 +681,11 @@ typedef void (show_value_ftype) (struct > instead print the value out directly. */ > extern show_value_ftype deprecated_show_value_hack; > > +/* Various sets of extra literals accepted. */ > +extern const literal_def integer_unlimited_literals[]; > +extern const literal_def uinteger_unlimited_literals[]; > +extern const literal_def zuinteger_unlimited_literals[]; > + > extern set_show_commands add_setshow_enum_cmd > (const char *name, command_class theclass, const char *const *enumlist, > const char **var, const char *set_doc, const char *show_doc, > @@ -748,6 +782,20 @@ extern set_show_commands add_setshow_opt > cmd_list_element **show_list); > > extern set_show_commands add_setshow_integer_cmd > + (const char *name, command_class theclass, int *var, > + const literal_def *extra_literals, const char *set_doc, > + const char *show_doc, const char *help_doc, cmd_func_ftype *set_func, > + show_value_ftype *show_func, cmd_list_element **set_list, > + cmd_list_element **show_list); > + > +extern set_show_commands add_setshow_integer_cmd > + (const char *name, command_class theclass, const literal_def *extra_literals, > + const char *set_doc, const char *show_doc, const char *help_doc, > + setting_func_types::set set_func, > + setting_func_types::get get_func, show_value_ftype *show_func, > + cmd_list_element **set_list, cmd_list_element **show_list); > + > +extern set_show_commands add_setshow_integer_cmd > (const char *name, command_class theclass, int *var, const char *set_doc, > const char *show_doc, const char *help_doc, cmd_func_ftype *set_func, > show_value_ftype *show_func, cmd_list_element **set_list, > @@ -760,6 +808,34 @@ extern set_show_commands add_setshow_int > setting_func_types::get get_func, show_value_ftype *show_func, > cmd_list_element **set_list, cmd_list_element **show_list); > > +extern set_show_commands add_setshow_pinteger_cmd > + (const char *name, command_class theclass, int *var, > + const literal_def *extra_literals, const char *set_doc, > + const char *show_doc, const char *help_doc, cmd_func_ftype *set_func, > + show_value_ftype *show_func, cmd_list_element **set_list, > + cmd_list_element **show_list); > + > +extern set_show_commands add_setshow_pinteger_cmd > + (const char *name, command_class theclass, const literal_def *extra_literals, > + const char *set_doc, const char *show_doc, const char *help_doc, > + setting_func_types::set set_func, > + setting_func_types::get get_func, show_value_ftype *show_func, > + cmd_list_element **set_list, cmd_list_element **show_list); > + > +extern set_show_commands add_setshow_uinteger_cmd > + (const char *name, command_class theclass, unsigned int *var, > + const literal_def *extra_literals, > + const char *set_doc, const char *show_doc, const char *help_doc, > + cmd_func_ftype *set_func, show_value_ftype *show_func, > + cmd_list_element **set_list, cmd_list_element **show_list); > + > +extern set_show_commands add_setshow_uinteger_cmd > + (const char *name, command_class theclass, const literal_def *extra_literals, > + const char *set_doc, const char *show_doc, const char *help_doc, > + setting_func_types::set set_func, > + setting_func_types::get get_func, show_value_ftype *show_func, > + cmd_list_element **set_list, cmd_list_element **show_list); > + > extern set_show_commands add_setshow_uinteger_cmd > (const char *name, command_class theclass, unsigned int *var, > const char *set_doc, const char *show_doc, const char *help_doc, > Index: src/gdb/guile/scm-param.c > =================================================================== > --- src.orig/gdb/guile/scm-param.c > +++ src/gdb/guile/scm-param.c > @@ -117,18 +117,33 @@ struct param_smob > static setting > make_setting (param_smob *s) > { > - if (var_type_uses (s->type)) > - return setting (s->type, &s->value.boolval); > - else if (var_type_uses (s->type)) > - return setting (s->type, &s->value.intval); > - else if (var_type_uses (s->type)) > - return setting (s->type, &s->value.autoboolval); > - else if (var_type_uses (s->type)) > - return setting (s->type, &s->value.uintval); > - else if (var_type_uses (s->type)) > - return setting (s->type, s->value.stringval); > - else if (var_type_uses (s->type)) > - return setting (s->type, &s->value.cstringval); > + enum var_types type = s->type; > + > + switch (type) > + { > + case var_zinteger: > + type = var_integer; > + break; > + case var_zuinteger: > + type = var_uinteger; > + break; > + case var_zuinteger_unlimited: > + type = var_pinteger; > + break; > + } > + > + if (var_type_uses (type)) > + return setting (type, &s->value.boolval); > + else if (var_type_uses (type)) > + return setting (type, &s->value.intval); > + else if (var_type_uses (type)) > + return setting (type, &s->value.autoboolval); > + else if (var_type_uses (type)) > + return setting (type, &s->value.uintval); > + else if (var_type_uses (type)) > + return setting (type, s->value.stringval); > + else if (var_type_uses (type)) > + return setting (type, &s->value.cstringval); > else > gdb_assert_not_reached ("unhandled var type"); > } > @@ -588,10 +603,6 @@ pascm_param_type_name (enum var_types pa > static SCM > pascm_param_value (const setting &var, int arg_pos, const char *func_name) > { > - /* Note: We *could* support var_integer here in case someone is trying to get > - the value of a Python-created parameter (which is the only place that > - still supports var_integer). To further discourage its use we do not. */ > - > switch (var.type ()) > { > case var_string: > @@ -631,20 +642,35 @@ pascm_param_value (const setting &var, i > return auto_keyword; > } > > - case var_zuinteger_unlimited: > - if (var.get () == -1) > - return unlimited_keyword; > - gdb_assert (var.get () >= 0); > - /* Fall through. */ > - case var_zinteger: > - return scm_from_int (var.get ()); > - > case var_uinteger: > - if (var.get ()== UINT_MAX) > - return unlimited_keyword; > - /* Fall through. */ > - case var_zuinteger: > - return scm_from_uint (var.get ()); > + case var_integer: > + case var_pinteger: > + { > + LONGEST value > + = (var.type () == var_uinteger > + ? static_cast (var.get ()) > + : static_cast (var.get ())); > + > + bool literal = false; > + if (var.extra_literals () != nullptr) > + for (const literal_def *l = var.extra_literals (); > + l->literal != nullptr; > + l++) > + if (value == l->use) > + { > + if (strcmp (l->literal, "unlimited") == 0) > + return unlimited_keyword; > + value = l->val; > + literal = true; > + } > + if (var.type () == var_pinteger && !literal) > + gdb_assert (value >= 0); > + > + if (var.type () == var_uinteger) > + return scm_from_uint (static_cast (value)); > + else > + return scm_from_int (static_cast (value)); > + } > > default: > break; > @@ -666,14 +692,14 @@ pascm_set_param_value_x (param_smob *p_s > { > setting var = make_setting (p_smob); > > - switch (var.type ()) > + switch (p_smob->type) > { > case var_string: > case var_string_noescape: > case var_optional_filename: > case var_filename: > SCM_ASSERT_TYPE (scm_is_string (value) > - || (var.type () != var_filename > + || (p_smob->type != var_filename > && gdbscm_is_false (value)), > value, arg_pos, func_name, > _("string or #f for non-PARAM_FILENAME parameters")); > @@ -739,8 +765,8 @@ pascm_set_param_value_x (param_smob *p_s > case var_uinteger: > case var_zuinteger: > case var_zuinteger_unlimited: > - if (var.type () == var_uinteger > - || var.type () == var_zuinteger_unlimited) > + if (p_smob->type == var_uinteger > + || p_smob->type == var_zuinteger_unlimited) > { > SCM_ASSERT_TYPE (gdbscm_is_bool (value) > || scm_is_eq (value, unlimited_keyword), > @@ -748,7 +774,7 @@ pascm_set_param_value_x (param_smob *p_s > _("integer or #:unlimited")); > if (scm_is_eq (value, unlimited_keyword)) > { > - if (var.type () == var_uinteger) > + if (p_smob->type == var_uinteger) > var.set (UINT_MAX); > else > var.set (-1); > @@ -761,12 +787,12 @@ pascm_set_param_value_x (param_smob *p_s > _("integer")); > } > > - if (var.type () == var_uinteger > - || var.type () == var_zuinteger) > + if (p_smob->type == var_uinteger > + || p_smob->type == var_zuinteger) > { > unsigned int u = scm_to_uint (value); > > - if (var.type () == var_uinteger && u == 0) > + if (p_smob->type == var_uinteger && u == 0) > u = UINT_MAX; > var.set (u); > } > @@ -774,7 +800,7 @@ pascm_set_param_value_x (param_smob *p_s > { > int i = scm_to_int (value); > > - if (var.type () == var_zuinteger_unlimited && i < -1) > + if (p_smob->type == var_zuinteger_unlimited && i < -1) > { > gdbscm_out_of_range_error (func_name, arg_pos, value, > _("must be >= -1")); > Index: src/gdb/maint-test-options.c > =================================================================== > --- src.orig/gdb/maint-test-options.c > +++ src/gdb/maint-test-options.c > @@ -207,6 +207,7 @@ static const gdb::option::option_def tes > gdb::option::uinteger_option_def { > "uinteger", > [] (test_options_opts *opts) { return &opts->uint_opt; }, > + uinteger_unlimited_literals, > nullptr, /* show_cmd_cb */ > N_("A uinteger option."), > nullptr, /* show_doc */ > @@ -214,9 +215,10 @@ static const gdb::option::option_def tes > }, > > /* A zuinteger_unlimited option. */ > - gdb::option::zuinteger_unlimited_option_def { > + gdb::option::integer_option_def { > "zuinteger-unlimited", > [] (test_options_opts *opts) { return &opts->zuint_unl_opt; }, > + zuinteger_unlimited_literals, > nullptr, /* show_cmd_cb */ > N_("A zuinteger-unlimited option."), > nullptr, /* show_doc */ > Index: src/gdb/python/py-param.c > =================================================================== > --- src.orig/gdb/python/py-param.c > +++ src/gdb/python/py-param.c > @@ -96,18 +96,33 @@ struct parmpy_object > static setting > make_setting (parmpy_object *s) > { > - if (var_type_uses (s->type)) > - return setting (s->type, &s->value.boolval); > - else if (var_type_uses (s->type)) > - return setting (s->type, &s->value.intval); > - else if (var_type_uses (s->type)) > - return setting (s->type, &s->value.autoboolval); > - else if (var_type_uses (s->type)) > - return setting (s->type, &s->value.uintval); > - else if (var_type_uses (s->type)) > - return setting (s->type, s->value.stringval); > - else if (var_type_uses (s->type)) > - return setting (s->type, &s->value.cstringval); > + enum var_types type = s->type; > + > + switch (type) > + { > + case var_zinteger: > + type = var_integer; > + break; > + case var_zuinteger: > + type = var_uinteger; > + break; > + case var_zuinteger_unlimited: > + type = var_pinteger; > + break; > + } > + > + if (var_type_uses (type)) > + return setting (type, &s->value.boolval); > + else if (var_type_uses (type)) > + return setting (type, &s->value.intval); > + else if (var_type_uses (type)) > + return setting (type, &s->value.autoboolval); > + else if (var_type_uses (type)) > + return setting (type, &s->value.uintval); > + else if (var_type_uses (type)) > + return setting (type, s->value.stringval); > + else if (var_type_uses (type)) > + return setting (type, &s->value.cstringval); > else > gdb_assert_not_reached ("unhandled var type"); > } > Index: src/gdb/python/python.c > =================================================================== > --- src.orig/gdb/python/python.c > +++ src/gdb/python/python.c > @@ -502,35 +502,34 @@ gdbpy_parameter_value (const setting &va > Py_RETURN_NONE; > } > > - case var_integer: > - if (var.get () == INT_MAX) > - Py_RETURN_NONE; > - /* Fall through. */ > - case var_zinteger: > - return gdb_py_object_from_longest (var.get ()).release (); > - > - case var_zuinteger_unlimited: > - { > - int val = var.get (); > - > - if (val == -1) > - Py_RETURN_NONE; > - return gdb_py_object_from_longest (val).release (); > - } > - > case var_uinteger: > + case var_integer: > + case var_pinteger: > { > - unsigned int val = var.get (); > + LONGEST value > + = (var.type () == var_uinteger > + ? static_cast (var.get ()) > + : static_cast (var.get ())); > > - if (val == UINT_MAX) > - Py_RETURN_NONE; > - return gdb_py_object_from_ulongest (val).release (); > - } > + if (var.extra_literals () != nullptr) > + for (const literal_def *l = var.extra_literals (); > + l->literal != nullptr; > + l++) > + if (value == l->use) > + { > + if (strcmp (l->literal, "unlimited") == 0) > + Py_RETURN_NONE; > + value = l->val; > + } > > - case var_zuinteger: > - { > - unsigned int val = var.get (); > - return gdb_py_object_from_ulongest (val).release (); > + if (var.type () == var_uinteger) > + return > + gdb_py_object_from_ulongest > + (static_cast (value)).release (); > + else > + return > + gdb_py_object_from_longest > + (static_cast (value)).release (); > } > } > > Index: src/gdb/valprint.c > =================================================================== > --- src.orig/gdb/valprint.c > +++ src/gdb/valprint.c > @@ -2974,8 +2974,8 @@ using boolean_option_def > = gdb::option::boolean_option_def; > using uinteger_option_def > = gdb::option::uinteger_option_def; > -using zuinteger_unlimited_option_def > - = gdb::option::zuinteger_unlimited_option_def; > +using integer_option_def > + = gdb::option::integer_option_def; > > /* Definitions of options for the "print" and "compile print" > commands. */ > @@ -3011,15 +3011,17 @@ static const gdb::option::option_def val > uinteger_option_def { > "elements", > [] (value_print_options *opt) { return &opt->print_max; }, > + uinteger_unlimited_literals, > show_print_max, /* show_cmd_cb */ > N_("Set limit on string chars or array elements to print."), > N_("Show limit on string chars or array elements to print."), > N_("\"unlimited\" causes there to be no limit."), > }, > > - zuinteger_unlimited_option_def { > + integer_option_def { > "max-depth", > [] (value_print_options *opt) { return &opt->max_depth; }, > + zuinteger_unlimited_literals, > show_print_max_depth, /* show_cmd_cb */ > N_("Set maximum print depth for nested structures, unions and arrays."), > N_("Show maximum print depth for nested structures, unions, and arrays."), > @@ -3079,6 +3081,7 @@ pretty-printers for that value.") > uinteger_option_def { > "repeats", > [] (value_print_options *opt) { return &opt->repeat_count_threshold; }, > + uinteger_unlimited_literals, > show_repeat_count_threshold, /* show_cmd_cb */ > N_("Set threshold for repeated print elements."), > N_("Show threshold for repeated print elements."),