Re: [PATCH v3] Make '{add-,}symbol-file' not care about the position of command line arguments

Pedro Alves
2017-11-30T15:01:32Z

On 11/30/2017 01:33 PM, Sergio Durigan Junior wrote:

> +# Since we're here, might as well test the 'symbol-file' command and
> +# if its arguments can also be passed at any position.
> +gdb_test "symbol-file -readnow $binfile" \
> + "Reading symbols from ${binfile}\.\.\.expanding to full symbols\.\.\.done\." \
> + "symbol-file with -readnow first"
> +clean_restart
> +gdb_test "symbol-file $binfile -readnow" \
> + "Reading symbols from ${binfile}\.\.\.expanding to full symbols\.\.\.done\." \
> + "symbol-file with -readnow second"
> +gdb_test "symbol-file -readnow" \
> + "no symbol file name was specified" \
> + "symbol-file without filename"
> +gdb_test "symbol-file -- non-existent-file" \
> + "non-existent-file: No such file or directory\." \
> + "symbol-file with -- disables option processing"

This should be "-non-existent-file", with leading "-" ...

Missing the same test with "add-symbol-file".

Another thing I wondered is why do we have the
expecting_sec_name/expecting_sec_addr variables instead of
simply reading the arguments in the argv ahead inline
like maintenance_print_symbols does?

As is, the patch essentially forbids section names
that start with '-':

(gdb) add-symbol-file ./gdb 0 -s -funnysection 0
add symbol table from file "./gdb" at
.text_addr = 0x0
-funnysection_addr = 0x0
(y or n)

=>

(gdb) add-symbol-file ./gdb 0 -s -funnysection 0
Missing section address after "-s"

and I'm not sure that's really a good idea. I don't think
we should really interpret/require section names to be in any
way/format.

(Another thing that I noticed but I'm kind of ignoring is the fact
that gdb_test treats the question as optional gdb output, so
pedantically gdb could stop outputting the question and answer
"n" automatically and the testcase wouldn't notice.)

Thanks,
Pedro Alves