This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] fix error checking on add-symbol-file command.
- From: Pedro Alves <palves at redhat dot com>
- To: Muhammad Bilal <mbilal at codesourcery dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Mon, 02 Sep 2013 19:43:10 +0100
- Subject: Re: [patch] fix error checking on add-symbol-file command.
- Authentication-results: sourceware.org; auth=none
- References: <52248FE1 dot 8030800 at codesourcery dot com> <52249253 dot 6060901 at codesourcery 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, §_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