[RFA,1/2] Fix add-symbol-file usage and errors

  This patch updaes add-symbol-file help and error text.

It changes add-symbol-file to throw an exception if "-s" is seen but
not all of the arguments are given.  Previously this was silently
ignored.

It changes the unrecognized argument message to more clearly state
what went wrong.

Finally, it updates the usage line in the help text to follow GNU
style regarding "metasyntactic variables"; a change I believe should
be made to all gdb help messages.

ChangeLog
2017-11-04  Tom Tromey  <tom@tromey.com>

	* symfile.c (add_symbol_file_command): Error if some arguments to
	-s are missing.  Change unrecognized-argument error message.
	(_initialize_symfile): Fix usage text for add-symbol-file.

testsuite/ChangeLog
2017-11-04  Tom Tromey  <tom@tromey.com>

	* gdb.base/relocate.exp: Update invalid argument test.
---
 gdb/ChangeLog                       |  6 ++++++
 gdb/symfile.c                       | 12 ++++++++----
 gdb/testsuite/ChangeLog             |  4 ++++
 gdb/testsuite/gdb.base/relocate.exp |  5 +++--
 4 files changed, 21 insertions(+), 6 deletions(-)
  

Hi Tom,

This looks mostly good to me, except the two issues pointed out
below.  Fix those that this is good to me.

On 11/04/2017 10:34 PM, Tom Tromey wrote:

> @@ -2261,6 +2260,11 @@ add_symbol_file_command (const char *args, int from_tty)
>      error (_("The address where %s has been loaded is missing"),
>  	   filename.get ());
>  
> +  if (expecting_sec_name)
> +    error (_("Missing section name after \"-s\""));
> +  else if (expecting_sec_addr)
> +    error (_("Missing section address after \"-s\""));

Could you add tests for these?

> +
>    /* Print the prompt for the query below.  And save the arguments into
>       a sect_addr_info structure to be passed around to other
>       functions.  We have to split this up into separate print
> @@ -3883,8 +3887,8 @@ to execute."), &cmdlist);
>  
>    c = add_cmd ("add-symbol-file", class_files, add_symbol_file_command, _("\
>  Load symbols from FILE, assuming FILE has been dynamically loaded.\n\
> -Usage: add-symbol-file FILE ADDR [-s <SECT> <SECT_ADDR> -s <SECT> <SECT_ADDR>\
> - ...]\nADDR is the starting address of the file's text.\n\
> +Usage: add-symbol-file FILE ADDR [-readnow | -s SECT SECT_ADDR]...\n\
> +ADDR is the starting address of the file's text.\n\
>  The optional arguments are section-name section-address pairs and\n\

It feels like this "The optional arguments are section..." sentence
should be adjusted too, since "-readnow" is also optional.  Agree?

>  should be specified if the data and bss segments are not contiguous\n\
>  with the text.  SECT is a section name to be loaded at SECT_ADDR."),
> diff --git a/gdb/testsuite/gdb.base/relocate.exp b/gdb/testsuite/gdb.base/relocate.exp
> index 5639cc8847..00327f5026 100644
> --- a/gdb/testsuite/gdb.base/relocate.exp
> +++ b/gdb/testsuite/gdb.base/relocate.exp
> @@ -31,9 +31,10 @@ gdb_reinitialize_dir $srcdir/$subdir
>  
>  #Check that invalid options are rejected.
>  foreach x {"-raednow" "readnow" "foo" "-readnow s"} {
> +    set word [lindex $x [expr [llength $x]-1]]
>      gdb_test "add-symbol-file ${binfile} 0 $x" \
> -	"USAGE: add-symbol-file <filename> <textaddress>.*-readnow.*-s <secname> <addr>.*" \
> -	"add-symbol-file: unknown option $x"
> +	"Unrecognized argument \"$word\"" \
> +	"add-symbol-file: unknown option $word"
>  }

Thanks,
Pedro Alves