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: Muhammad Bilal <mbilal at codesourcery dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Tue, 3 Sep 2013 16:07:23 +0500
- 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> <5224DC3E dot 2060909 at redhat dot com>
On 09/02/2013 11:43 PM, Pedro Alves wrote:
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
Could you try that out?
yes, it is good
Typo "unknown". EMISSINGVERB, really. Perhaps:
* symfile.c (add_symbol_file_command): Error out on unknown
option.
fixed.
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:
fixed.
Thanks for review.
Please find attached updated patch.
2013-09-03 Muhammad Bilal <mbilal@codesourcery.com>
Pedro Alves <palves@redhat.com>
* gdb.base/relocate.exp: Check that invalid options are
rejected.
2013-09-03 Muhammad Bilal <mbilal@codesourcery.com>
Pedro Alves <palves@redhat.com>
* symfile.c (add_symbol_file_command): Error out on unknown
option.
Handling EXPECTING_SEC_* before '-' options and collapse
into single conditional branch.
Thanks,
-Bilal
Index: ../src/gdb/symfile.c
===================================================================
RCS file: /cvs/src/src/gdb/symfile.c,v
retrieving revision 1.384
diff -u -p -r1.384 symfile.c
--- ../src/gdb/symfile.c 20 Aug 2013 15:04:51 -0000 1.384
+++ ../src/gdb/symfile.c 3 Sep 2013 10:45:46 -0000
@@ -2214,63 +2214,55 @@ add_symbol_file_command (char *args, int
filename = tilde_expand (arg);
make_cleanup (xfree, filename);
}
+ else if (argcnt == 1)
+ {
+ /* The second argument is always the text address at which
+ to load the program. */
+ sect_opts[section_index].name = ".text";
+ sect_opts[section_index].value = arg;
+ 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 (argcnt == 1)
- {
- /* The second argument is always the text address at which
- to load the program. */
- sect_opts[section_index].name = ".text";
- sect_opts[section_index].value = arg;
- 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
- {
- /* It's an option (starting with '-') or it's an argument
- to an option. */
-
- if (*arg == '-')
- {
- if (strcmp (arg, "-readnow") == 0)
- flags |= OBJF_READNOW;
- else if (strcmp (arg, "-s") == 0)
- {
- expecting_sec_name = 1;
- expecting_sec_addr = 1;
- }
- }
- else
- {
- 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
- error (_("USAGE: add-symbol-file <filename> <textaddress>"
- " [-readnow] [-s <secname> <addr>]*"));
- }
- }
+ {
+ /* It's an option (starting with '-') or it's an argument
+ to an option. */
+
+ 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>]*"));
+ }
}
/* This command takes at least two arguments. The first one is a
Index: ../src/gdb/testsuite/gdb.base/relocate.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/relocate.exp,v
retrieving revision 1.20
diff -u -p -r1.20 relocate.exp
--- ../src/gdb/testsuite/gdb.base/relocate.exp 27 Jun 2013 18:53:49 -0000 1.20
+++ ../src/gdb/testsuite/gdb.base/relocate.exp 3 Sep 2013 10:45:46 -0000
@@ -51,6 +51,12 @@ proc get_var_address { var } {
gdb_exit
gdb_start
gdb_reinitialize_dir $srcdir/$subdir
+#Check that invalid options are rejected.
+foreach x {"-raednow" "readnow" "foo" "-readnow s"} {
+ gdb_test "add-symbol-file ${binfile} 0 $x" \
+ "USAGE: add-symbol-file <filename> <textaddress>.*-readnow.*-s <secname> <addr>.*" \
+ "unknow option $x"
+}
# Load the object file.
gdb_test "add-symbol-file ${binfile} 0" \