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

[Pedro Alves <palves at redhat dot com>]

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