Re: [patch] fix error checking on add-symbol-file command.

[Pedro Alves <palves at redhat dot com>]

On 09/02/2013 02:27 PM, Muhammad Bilal wrote:
> Hi,
> 
> GDB does not recognizes the bad option on add-symbol-file command.

Thanks.

> 
> currently GDB shows
> 
> (gdb) add-symbol-file test 0 -readnow -blabla
> add symbol table from file "test" at
>      .text_addr = 0x0
> (y or n) y
> Reading symbols from /home/mbilal/test...done.
> (gdb)
> 
> 
> expected result
> 
>   (gdb) add-symbol-file test 0 -readnow -blabla
> unknown option '-blabla'
> (gdb)

I notice the other branch throws:

		    error (_("USAGE: add-symbol-file <filename> <textaddress>"
			     " [-readnow] [-s <secname> <addr>]*"));

and wonder whether it wouldn't be better to instead
make that error reachable from both branches.  That'd just
mean the if/if/else/if nesting would be collapsed.
Also considering that there are targets with negative
addresses (such as MIPS), that suggests always handling
EXPECTING_SEC_* before the options, so that a '-' negative
address wouldn't be confused with an option.  All in all,
that means doing this instead:

	if (expecting_sec_name)
	  {
	    sect_opts[section_index].name = arg;
	    expecting_sec_name = 0;
	  }
	else if (expecting_sec_addr)
	  {
	    sect_opts[section_index].value = arg;
	    expecting_sec_addr = 0;
	    if (++section_index >= num_sect_opts)
	      {
		num_sect_opts *= 2;
		sect_opts = ((struct sect_opt *)
			       xrealloc (sect_opts,
					 num_sect_opts
					 * sizeof (struct sect_opt)));
	      }
	  }
	else if (strcmp (arg, "-readnow") == 0)
	  flags |= OBJF_READNOW;
	else if (strcmp (arg, "-s") == 0)
	  {
	    expecting_sec_name = 1;
	    expecting_sec_addr = 1;
	  }
	else
	  error (_("USAGE: add-symbol-file <filename> <textaddress>"
			     " [-readnow] [-s <secname> <addr>]*"));

Could you try that out?

BTW, I noticed that sect_opts leaks on error.  A cleanup
like:

 make_cleanup (free_current_contents, &sect_opts);

is missing...  (though that'd be a separate change.)

> 2013-09-02  Muhammad Bilal <mbilal@codesourcery.com>
>
>          * symfile.c (add_symbol_file_command): Error checking on
>          unkown option.

Typo "unknown".  EMISSINGVERB, really.  Perhaps:

        * symfile.c (add_symbol_file_command): Error out on unknown
	option.


>
>  # Load the object file.
> +gdb_test "add-symbol-file ${binfile} 0 -blabla" "unknown option.*-blabla.*" "add-symbol-file error checking"
>  gdb_test "add-symbol-file ${binfile} 0" \

This is not really loading the file, so put it above
the comment.  Also I really suggest tweaking the message -- there
are many kinds of errors, and this is specifically about user
input error:

# Check that invalid options are rejected.
gdb_test "add-symbol-file ${binfile} 0 -blabla" \
	"unknown option.*-blabla.*" \
	"add-symbol-file rejects unknown option"

# Load the object file.
gdb_test "add-symbol-file ${binfile} 0" \

-- 
Pedro Alves