Re: [PATCH v2 2/2] Make gdbserver work with filename-only binaries

[Pedro Alves <palves@redhat.com>]

Hi Sergio,

A few quick comments below.  

> @@ -3539,6 +3564,13 @@ captured_main (int argc, char *argv[])
>    const char *selftest_filter = NULL;
>  #endif
>  
> +  current_directory = getcwd (NULL, 0);
> +  if (current_directory == NULL)
> +    {
> +      warning (_("%s: error finding working directory"),
> +	       safe_strerror (errno));

I think it's much more usual to put the strerror string at the
end of the warning rather than at the beginning.

I.e., something like:

   warning (_("Could not find working directory: %s"),
       safe_strerror (errno));

See

  $ (cd gdb; git grep -3 "warning (" *.c | grep strerr -C 3)

for example.

From the ongoing discussion, it sounded like this hunk may change
in the next iteration, but I thought I'd still comment as it
may help with future patches.


> +    }


> +# We only test things locally, and on native-gdbserver
> +if { [is_remote target] || [is_remote host] || ![use_gdb_stub] } {
> +    return 0
> +}


I don't see why to restrict this to "only on native-gdbserver".  The test
is calling gdbserver_start etc. manually, so it should work when testing
with any local board, I think?  I.e., when testing with native or
extended-remote too.  For the latter, tests will usually call "disconnect".

> +
> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } {
> +    return -1
> +}
> +
> +set target_exec [gdbserver_download_current_prog]
> +set target_execname [file tail $target_exec]
> +# We temporarily copy the file to our current directory
> +file copy -force $target_exec [pwd]
> +set res [gdbserver_start "" $target_execname]

Please remind me -- is the current directory here usually
the testcase's output dir?  I.e., is it guaranteed that
the current directory here is not going to be the same
directory of another testcase running in parallel at
the same time?

> +
> +set gdbserver_protocol [lindex $res 0]
> +set gdbserver_gdbport [lindex $res 1]
> +gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport
> +
> +if { [runto_main] } {
> +    pass "load filename without absolute path"
> +} else {
> +    fail "load filename without absolute path"
> +}

runto_main here looks a bit odd to me.  You're manually connecting
with gdb_target_cmd, bypassing whatever the current board file
may want to do, but then you're using a routine that call back
into the board file to do random things to prepare for running.

I think you should set a breakpoint at main and continue to
it without using runto_main.  Note how no other test in gdb.server/
uses runto_main.

Thanks,
Pedro Alves