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

[Pedro Alves <palves@redhat.com>]

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