Avoid spaces in osabi names

Message ID 1457377364-990-1-git-send-email-palves@redhat.com
State New, archived
Headers

Commit Message

Pedro Alves March 7, 2016, 7:02 p.m. UTC
  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.

gdb/ChangeLog:
2016-03-07  Pedro Alves  <palves@redhat.com>

	* osabi.c (gdb_osabi_names): Avoid spaces in osabi names.
---
 gdb/osabi.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)
  

Comments

Simon Marchi March 7, 2016, 7:12 p.m. UTC | #1
On 16-03-07 02: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.
> 
> gdb/ChangeLog:
> 2016-03-07  Pedro Alves  <palves@redhat.com>
> 
> 	* osabi.c (gdb_osabi_names): Avoid spaces in osabi names.
> ---
>  gdb/osabi.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/gdb/osabi.c b/gdb/osabi.c
> index e8a77ab..f7d4e74 100644
> --- a/gdb/osabi.c
> +++ b/gdb/osabi.c
> @@ -64,17 +64,17 @@ static const struct osabi_names gdb_osabi_names[] =
>    { "GNU/Hurd", NULL },
>    { "Solaris", NULL },
>    { "GNU/Linux", "linux(-gnu)?" },
> -  { "FreeBSD a.out", NULL },
> -  { "FreeBSD ELF", NULL },
> -  { "NetBSD a.out", NULL },
> -  { "NetBSD ELF", NULL },
> -  { "OpenBSD ELF", NULL },
> -  { "Windows CE", NULL },
> +  { "FreeBSD/a.out", NULL },
> +  { "FreeBSD/ELF", NULL },
> +  { "NetBSD/a.out", NULL },
> +  { "NetBSD/ELF", NULL },
> +  { "OpenBSD/ELF", NULL },
> +  { "WindowsCE", NULL },
>    { "DJGPP", NULL },
>    { "Irix", NULL },
> -  { "HP/UX ELF", NULL },
> -  { "HP/UX SOM", NULL },
> -  { "QNX Neutrino", NULL },
> +  { "HP-UX/ELF", NULL },
> +  { "HP-UX/SOM", NULL },

We nuked hp-ux some time ago.  Is that some leftover?
  
Joel Brobecker March 8, 2016, 8:59 p.m. UTC | #2
Hi Pedro,

> gdb/ChangeLog:
> 2016-03-07  Pedro Alves  <palves@redhat.com>
> 
> 	* osabi.c (gdb_osabi_names): Avoid spaces in osabi names.

FWIW, I think this is the best compromise. The new names you chose
also look good to me.
  
Pedro Alves March 9, 2016, 3:58 p.m. UTC | #3
On 03/08/2016 08:59 PM, Joel Brobecker wrote:
> Hi Pedro,
> 
>> gdb/ChangeLog:
>> 2016-03-07  Pedro Alves  <palves@redhat.com>
>>
>> 	* osabi.c (gdb_osabi_names): Avoid spaces in osabi names.
> 
> FWIW, I think this is the best compromise. The new names you chose
> also look good to me.

Alright, pushed.  Thanks.

I should point out: the osabi can also be sent in XML target 
descriptions, but gdbserver is currently only including
an <osabi> element on GNU/Linux target descriptions.  Hopefully
no stub outside our tree is sending any of the affected names.

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/osabi.c b/gdb/osabi.c
index e8a77ab..f7d4e74 100644
--- a/gdb/osabi.c
+++ b/gdb/osabi.c
@@ -64,17 +64,17 @@  static const struct osabi_names gdb_osabi_names[] =
   { "GNU/Hurd", NULL },
   { "Solaris", NULL },
   { "GNU/Linux", "linux(-gnu)?" },
-  { "FreeBSD a.out", NULL },
-  { "FreeBSD ELF", NULL },
-  { "NetBSD a.out", NULL },
-  { "NetBSD ELF", NULL },
-  { "OpenBSD ELF", NULL },
-  { "Windows CE", NULL },
+  { "FreeBSD/a.out", NULL },
+  { "FreeBSD/ELF", NULL },
+  { "NetBSD/a.out", NULL },
+  { "NetBSD/ELF", NULL },
+  { "OpenBSD/ELF", NULL },
+  { "WindowsCE", NULL },
   { "DJGPP", NULL },
   { "Irix", NULL },
-  { "HP/UX ELF", NULL },
-  { "HP/UX SOM", NULL },
-  { "QNX Neutrino", NULL },
+  { "HP-UX/ELF", NULL },
+  { "HP-UX/SOM", NULL },
+  { "QNX-Neutrino", NULL },
   { "Cygwin", NULL },
   { "AIX", NULL },
   { "DICOS", NULL },