[PATCH] Make file transfer commands work with all (native) targets. (was: Re: [PATCH 04/16] push remote_desc into struct remote_state)

Pedro Alves
2013-06-28T16:34:05Z

On 06/27/2013 09:21 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

> Thanks Pedro.
>
> You saw the future quite accurately here. I did see crashes in this
> code after the multi-target conversion, precisely because I changed
> get_remote_state to return NULL when not connected; and this path is
> exercised by the test suite.
>
> So, a later patch will introduce the == NULL check as well.
>
> In this series, though, get_remote_state cannot return NULL. This may
> seem odd, but it is consistent with the state of the code before the
> series -- and I wanted to try to make this series obviously not
> behavior-changing.

Ack.

> Pedro> remote_file_get could nowadays be using the target_fileio_XXX methods
> Pedro> instead of remote_hostio_XXX, and therefore the command could be
> Pedro> generalized to work with all targets.

Actually, doing this is quite easy, so I went ahead, in the name
of local/remote feature parity. We can connect to a local gdbserver
and do file transfer in the local system; there seems to be no
reason we can't do it with native debugging too.

WDYT?

If this looks good, then a followup could move these routines
out of remote.c, and file-transfer.exp out of gdb.server/.

I considered aliasing "target put/get/delete" to "remote put/get/delete",
but didn't do it,as I didn't want to do too much in case you guys didn't
like this. The MI commands, and their description in the manual are
already target agnostic.

-----
From 85c29ce88ec7f0d3533a6f900fe36571135bd452 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 28 Jun 2013 16:00:16 +0100
Subject: [PATCH] Make file transfer commands work with all (native) targets.

This makes the file transfer commands use the generic target_fileio_
hooks, therefore making them work the native targets too.

Tested on x86_64 Fedora 17.

gdb/
2013-06-28 Pedro Alves <palves@redhat.com>

* NEWS: Mention that file transfer commands now work with native
targets.
* remote.c (remote_fileio_errno_to_host): Rename to ...
(fileio_errno_to_host_errno): ... this. Add comment.
(remote_hostio_error): Rename to ...
(fileio_error): ... this. Avoid saying "remote" in errors.
(remote_hostio_close_cleanup): Rename to ...
(target_fileio_close_cleanup): ... this. Use the target_fileio
interfaces rather than calling the remote methods directly.
(remote_bfd_iovec_open, remote_bfd_iovec_pread): Adjust.
(remote_file_put, remote_file_get, remote_file_delete): Use the
target_fileio interfaces rather than calling the remote methods
directly. Rename locals and parameters. Don't check if connected
to a remote target. Always try sending 4k bytes per transfer,
instead of consulting the packet size.

gdb/doc/
2013-06-28 Pedro Alves <palves@redhat.com>

* gdb.texinfo (File Transfer): Mention that the file transfer
commands also work with native targets.

gdb/testsuite/
2013-06-28 Pedro Alves <palves@redhat.com>

Make it work with all targets.
* gdb.server/file-transfer.exp: Don't load gdbserver-support.exp.
Don't check whether to skip gdbserver tests. Don't force
disconnect, and don't spawn gdbserver explicitly. Run to main if
testing with a remote stub.
---
gdb/NEWS | 8 +++
gdb/doc/gdb.texinfo | 4 ++
gdb/remote.c | 98 ++++++++++++++----------------
gdb/testsuite/gdb.server/file-transfer.exp | 20 +++---
4 files changed, 67 insertions(+), 63 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index e469f1e..2e035aa 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -70,6 +70,10 @@ show range-stepping
* The exception-related catchpoints, like "catch throw", now accept a
regular expression which can be used to filter exceptions by type.

+* The remote put/get/delete commands (file transfer) now work with
+ native targets too. Previously, they only worked with remote
+ targets.
+
* MI changes

** The -trace-save MI command can optionally save trace buffer in Common
@@ -84,6 +88,10 @@ show range-stepping
** The new command -trace-frame-collected dumps collected variables,
computed expressions, tvars, memory and registers in a traceframe.

+ ** The file transfer commands (-target-file-put, -target-file-get,
+ -target-file-delete) now work with native targets too. Previously,
+ they only worked with remote targets.
+
* New system-wide configuration scripts
A GDB installation now provides scripts suitable for use as system-wide
configuration scripts for the following systems:
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index d75a3d1..f270c87 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -17979,6 +17979,10 @@ the only way to upload or download files.

Not all remote targets support these commands.

+Although most useful with remote targets, note that these commands
+also work when native debugging. In that case, the host and target
+systems are the same system.
+
@table @code
@kindex remote put
@item remote put @var{hostfile} @var{targetfile}
diff --git a/gdb/remote.c b/gdb/remote.c
index a29fe23..672898d 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -9950,8 +9950,11 @@ remote_hostio_readlink (const char *filename, int *remote_errno)
return ret;
}

+/* Convert ERRNUM from "Hosted File I/O" error number to host
+ errno. */
+
static int
-remote_fileio_errno_to_host (int errnum)
+fileio_errno_to_host_errno (int errnum)
{
switch (errnum)
{
@@ -10002,23 +10005,23 @@ remote_fileio_errno_to_host (int errnum)
}

static char *
-remote_hostio_error (int errnum)
+fileio_error (int target_errno)
{
- int host_error = remote_fileio_errno_to_host (errnum);
+ int host_error = fileio_errno_to_host_errno (target_errno);

if (host_error == -1)
- error (_("Unknown remote I/O error %d"), errnum);
+ error (_("Unknown I/O error %d"), target_errno);
else
- error (_("Remote I/O error: %s"), safe_strerror (host_error));
+ error (_("I/O error: %s"), safe_strerror (host_error));
}

static void
-remote_hostio_close_cleanup (void *opaque)
+target_fileio_close_cleanup (void *opaque)
{
int fd = *(int *) opaque;
- int remote_errno;
+ int target_errno;

- remote_hostio_close (fd, &remote_errno);
+ target_fileio_close (fd, &target_errno);
}

@@ -10034,7 +10037,7 @@ remote_bfd_iovec_open (struct bfd *abfd, void *open_closure)
fd = remote_hostio_open (filename + 7, FILEIO_O_RDONLY, 0, &remote_errno);
if (fd == -1)
{
- errno = remote_fileio_errno_to_host (remote_errno);
+ errno = fileio_errno_to_host_errno (remote_errno);
bfd_set_error (bfd_error_system_call);
return NULL;
}
@@ -10078,7 +10081,7 @@ remote_bfd_iovec_pread (struct bfd *abfd, void *stream, void *buf,
break;
if (bytes == -1)
{
- errno = remote_fileio_errno_to_host (remote_errno);
+ errno = fileio_errno_to_host_errno (remote_errno);
bfd_set_error (bfd_error_system_call);
return -1;
}
@@ -10116,37 +10119,34 @@ remote_bfd_open (const char *remote_file, const char *target)
}

void
-remote_file_put (const char *local_file, const char *remote_file, int from_tty)
+remote_file_put (const char *local_file, const char *target_file, int from_tty)
{
struct cleanup *back_to, *close_cleanup;
- int retcode, fd, remote_errno, bytes, io_size;
+ int retcode, fd, target_errno, bytes, io_size;
FILE *file;
gdb_byte *buffer;
int bytes_in_buffer;
int saw_eof;
ULONGEST offset;

- if (!remote_desc)
- error (_("command can only be used with remote target"));
-
file = gdb_fopen_cloexec (local_file, "rb");
if (file == NULL)
perror_with_name (local_file);
back_to = make_cleanup_fclose (file);

- fd = remote_hostio_open (remote_file, (FILEIO_O_WRONLY | FILEIO_O_CREAT
+ fd = target_fileio_open (target_file, (FILEIO_O_WRONLY | FILEIO_O_CREAT
| FILEIO_O_TRUNC),
- 0700, &remote_errno);
+ 0700, &target_errno);
if (fd == -1)
- remote_hostio_error (remote_errno);
+ fileio_error (target_errno);

- /* Send up to this many bytes at once. They won't all fit in the
- remote packet limit, so we'll transfer slightly fewer. */
- io_size = get_remote_packet_size ();
+ /* Start by sending up to 16K at a time. The target will throttle
+ this number down if necessary. */
+ io_size = 16384;
buffer = xmalloc (io_size);
make_cleanup (xfree, buffer);

- close_cleanup = make_cleanup (remote_hostio_close_cleanup, &fd);
+ close_cleanup = make_cleanup (target_fileio_close_cleanup, &fd);

bytes_in_buffer = 0;
saw_eof = 0;
@@ -10178,13 +10178,13 @@ remote_file_put (const char *local_file, const char *remote_file, int from_tty)
bytes += bytes_in_buffer;
bytes_in_buffer = 0;

- retcode = remote_hostio_pwrite (fd, buffer, bytes,
- offset, &remote_errno);
+ retcode = target_fileio_pwrite (fd, buffer, bytes,
+ offset, &target_errno);

if (retcode < 0)
- remote_hostio_error (remote_errno);
+ fileio_error (target_errno);
else if (retcode == 0)
- error (_("Remote write of %d bytes returned 0!"), bytes);
+ error (_("Write of %d bytes returned 0!"), bytes);
else if (retcode < bytes)
{
/* Short write. Save the rest of the read data for the next
@@ -10197,8 +10197,8 @@ remote_file_put (const char *local_file, const char *remote_file, int from_tty)
}

discard_cleanups (close_cleanup);
- if (remote_hostio_close (fd, &remote_errno))
- remote_hostio_error (remote_errno);
+ if (target_fileio_close (fd, &target_errno))
+ fileio_error (target_errno);

if (from_tty)
printf_filtered (_("Successfully sent file \"%s\".\n"), local_file);
@@ -10206,43 +10206,40 @@ remote_file_put (const char *local_file, const char *remote_file, int from_tty)
}

void
-remote_file_get (const char *remote_file, const char *local_file, int from_tty)
+remote_file_get (const char *target_file, const char *local_file, int from_tty)
{
struct cleanup *back_to, *close_cleanup;
- int fd, remote_errno, bytes, io_size;
+ int fd, target_errno, bytes, io_size;
FILE *file;
gdb_byte *buffer;
ULONGEST offset;

- if (!remote_desc)
- error (_("command can only be used with remote target"));
-
- fd = remote_hostio_open (remote_file, FILEIO_O_RDONLY, 0, &remote_errno);
+ fd = target_fileio_open (target_file, FILEIO_O_RDONLY, 0, &target_errno);
if (fd == -1)
- remote_hostio_error (remote_errno);
+ fileio_error (target_errno);

file = gdb_fopen_cloexec (local_file, "wb");
if (file == NULL)
perror_with_name (local_file);
back_to = make_cleanup_fclose (file);

- /* Send up to this many bytes at once. They won't all fit in the
- remote packet limit, so we'll transfer slightly fewer. */
- io_size = get_remote_packet_size ();
+ /* Start by reading up to 16K at a time. The target will throttle
+ this number down if necessary. */
+ io_size = 16384;
buffer = xmalloc (io_size);
make_cleanup (xfree, buffer);

- close_cleanup = make_cleanup (remote_hostio_close_cleanup, &fd);
+ close_cleanup = make_cleanup (target_fileio_close_cleanup, &fd);

offset = 0;
while (1)
{
- bytes = remote_hostio_pread (fd, buffer, io_size, offset, &remote_errno);
+ bytes = target_fileio_pread (fd, buffer, io_size, offset, &target_errno);
if (bytes == 0)
/* Success, but no bytes, means end-of-file. */
break;
if (bytes == -1)
- remote_hostio_error (remote_errno);
+ fileio_error (target_errno);

offset += bytes;

@@ -10252,28 +10249,25 @@ remote_file_get (const char *remote_file, const char *local_file, int from_tty)
}

discard_cleanups (close_cleanup);
- if (remote_hostio_close (fd, &remote_errno))
- remote_hostio_error (remote_errno);
+ if (target_fileio_close (fd, &target_errno))
+ fileio_error (target_errno);

if (from_tty)
- printf_filtered (_("Successfully fetched file \"%s\".\n"), remote_file);
+ printf_filtered (_("Successfully fetched file \"%s\".\n"), target_file);
do_cleanups (back_to);
}

void
-remote_file_delete (const char *remote_file, int from_tty)
+remote_file_delete (const char *target_file, int from_tty)
{
- int retcode, remote_errno;
-
- if (!remote_desc)
- error (_("command can only be used with remote target"));
+ int retcode, target_errno;

- retcode = remote_hostio_unlink (remote_file, &remote_errno);
+ retcode = target_fileio_unlink (target_file, &target_errno);
if (retcode == -1)
- remote_hostio_error (remote_errno);
+ fileio_error (target_errno);

if (from_tty)
- printf_filtered (_("Successfully deleted file \"%s\".\n"), remote_file);
+ printf_filtered (_("Successfully deleted file \"%s\".\n"), target_file);
}

static void
diff --git a/gdb/testsuite/gdb.server/file-transfer.exp b/gdb/testsuite/gdb.server/file-transfer.exp
index aa56380..2e17216 100644
--- a/gdb/testsuite/gdb.server/file-transfer.exp
+++ b/gdb/testsuite/gdb.server/file-transfer.exp
@@ -16,23 +16,21 @@

# Test gdbserver monitor commands.

-load_lib gdbserver-support.exp
-
standard_testfile server.c

-if { [skip_gdbserver_tests] } {
- return 0
-}
-
if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
return -1
}

-# Make sure we're disconnected, in case we're testing with an
-# extended-remote board, therefore already connected.
-gdb_test "disconnect" ".*"
-
-gdbserver_run ""
+# If using a remote stub, then make sure we're connected, otherwise
+# fileio operations fall back to the native target. IOW, we'd test
+# the wrong target.
+if [target_info exists use_gdb_stub] {
+ if { ![runto_main] } then {
+ fail "Can't run to main"
+ return -1
+ }
+}

proc test_file_transfer { filename description } {
gdb_test "remote put \"$filename\" down-server" \
--
1.7.11.7