[PATCH] Don't allow whitespace in enum values of enum commands (Re: [PATCH] Avoid spaces in osabi names)

Pedro Alves
2016-12-09T16:57:20Z

Now that we don't have spaces in names anymore (and all-architectures.exp
is in, and a.out support is gone too), how about this patch below, to make
sure we don't introduce the problem in any other enum command?

On 03/07/2016 07:02 PM, Pedro Alves wrote:
> It's not possible today to select some of the osabis by name.
> Specifically, those that have spaces in their names and then the first
> word is ambiguous...
>
> For example:
> (gdb) set osabi <TAB>
> AIX
> Cygwin
> DICOS
> DJGPP
> Darwin
> FreeBSD ELF
> FreeBSD a.out
> GNU/Hurd
> GNU/Linux
> LynxOS178
> NetBSD ELF
> NetBSD a.out
> Newlib
> OpenBSD ELF
> OpenVMS
> QNX Neutrino
> SDE
> SVR4
> Solaris
> Symbian
> Windows CE
> auto
> default
> none
> (gdb) set osabi FreeBSD ELF
> Ambiguous item "FreeBSD ELF".
>
> In reality, because "set osabi" is an enum command, that was
> equivalent to trying "set osabi FreeBSD", which is then obviously
> ambiguous, because of "FreeBSD ELF" and "FreeBSD a.out".
>
> Also, even if the first word is not ambiguous, we actually ignore
> whatever comes after the first word:
>
> (gdb) set osabi GNU/Linux
> (gdb) show osabi
> The current OS ABI is "GNU/Linux".
> The default OS ABI is "GNU/Linux".
> (gdb) set osabi Windows SomeNonsense
> ^^^^^^^^^^^^
> (gdb) show osabi
> The current OS ABI is "Windows CE".
> The default OS ABI is "GNU/Linux".
> (gdb)
>
> Fix this by avoiding spaces in osabi names.
>
> We could instead make "set osabi" have a custom set hook, or
> alternatively make the enum set hook (in cli-setshow.c) handle values
> with spaces, but OTOH, I have a feeling that could cause trouble.
> E.g., in cases where we might want to write more than one enum value
> in the same line. We could support quoting as workaround, but, do we
> want that? "No spaces" seems simpler.
>
> I'm thinking that if we take this route, we should probably make the
> enum command registration functions assert that no possible enum value
> contains spaces. I tried that, and other than the "set architecture"
> command, I found no other case. I sent a patch to binutils@ to try to
> fix that one.

This is the patch that I had tried back then. I retested it on current
master, with --enable-targets=all (on x86_64 Fedora), and there are
no regressions.

From 44f14ec124d582f9fa5baa89c696fe4f23a4bb4e Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 7 Mar 2016 18:43:29 +0000
Subject: [PATCH] Don't allow whitespace in enum values of enum commands

Enum values with spaces don't work, so make sure we can't create them.

Ref: https://sourceware.org/ml/gdb-patches/2016-03/msg00116.html

gdb/ChangeLog:
2016-12-09 Pedro Alves <palves@redhat.com>

* cli/cli-decode.c (add_setshow_enum_cmd): Assert the enum values
do not include whitespace.
(complete_on_enum): Likewise.
---
gdb/cli/cli-decode.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index acc9c42..65bfb16 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -480,8 +480,9 @@ add_setshow_cmd_full (const char *name,

/* Add element named NAME to command list LIST (the list for set or
some sublist thereof). CLASS is as in add_cmd. ENUMLIST is a list
- of strings which may follow NAME. VAR is address of the variable
- which will contain the matching string (from ENUMLIST). */
+ of strings which may follow NAME. The list elements must not
+ include spaces. VAR is address of the variable which will contain
+ the matching string (from ENUMLIST). */

void
add_setshow_enum_cmd (const char *name,
@@ -498,6 +499,10 @@ add_setshow_enum_cmd (const char *name,
{
struct cmd_list_element *c;

+ /* Enum values must not include whitespace. */
+ for (size_t i = 0; enumlist[i] != NULL; i++)
+ gdb_assert (strpbrk (enumlist[i], " \t\n") == NULL);
+
add_setshow_cmd_full (name, theclass, var_enum, var,
set_doc, show_doc, help_doc,
set_func, show_func,
@@ -1866,6 +1871,9 @@ complete_on_enum (const char *const *enumlist,
{
char *match;

+ /* Enum values must not include whitespace. */
+ gdb_assert (strpbrk (name, " \t\n") == NULL);
+
match = (char *) xmalloc (strlen (word) + strlen (name) + 1);
if (word == text)
strcpy (match, name);
--
2.5.5