gdb/remote.c is presently handling partial memory reads itself.
It shouldn't. Higher layers should handle it. Consider, for
example target_read_string. If the remote end gives us less than
we wanted, we might as well look for the end \0 byte immediately
in the data we already have, rather than having remote.c possibly
forcing another unnecessary roundtrip until it has as much data
as requested.
Tested against an x86_64-linux gdbserver and checked in.
--
Pedro Alves
2011-01-25 Pedro Alves <pedro@codesourcery.com>
Stop remote_read_bytes from handling partial reads itself.
* remote-fileio.c: Include target.h.
(remote_fileio_write_bytes): Delete.
(remote_fileio_func_open, remote_fileio_func_write)
(remote_fileio_func_rename, remote_fileio_func_unlink): Use
target_read_memory.
(remote_fileio_func_stat): Use target_read_memory and
target_write_memory.
(remote_fileio_func_gettimeofday): Use target_write_memory.
(remote_fileio_func_system): Use target_read_memory.
* remote.c (remote_write_bytes): Make it static.
(remote_read_bytes): Don't handle partial reads here.
* remote.h (remote_read_bytes): Delete declaration.
---
gdb/remote-fileio.c | 76 +++++++++++++-----------------------------
gdb/remote.c | 92
+++++++++++++++++++---------------------------------
gdb/remote.h | 5 --
3 files changed, 58 insertions(+), 115 deletions(-)
Index: src/gdb/remote-fileio.c
===================================================================
--- src.orig/gdb/remote-fileio.c 2011-01-24 12:42:54.126176998 +0000
+++ src/gdb/remote-fileio.c 2011-01-25 11:40:48.997639995 +0000
@@ -30,6 +30,7 @@
#include "exceptions.h"
#include "remote-fileio.h"
#include "event-loop.h"
+#include "target.h"
#include <fcntl.h>
#include <sys/time.h>
@@ -588,29 +589,11 @@ remote_fileio_return_success (int retcod
remote_fileio_reply (retcode, 0);
}
-/* Wrapper function for remote_write_bytes() which has the disadvantage to
- write only one packet, regardless of the requested number of bytes to
- transfer. This wrapper calls remote_write_bytes() as often as needed. */
-static int
-remote_fileio_write_bytes (CORE_ADDR memaddr, gdb_byte *myaddr, int len)
-{
- int ret = 0, written;
-
- while (len > 0 && (written = remote_write_bytes (memaddr, myaddr, len)) >
0)
- {
- len -= written;
- memaddr += written;
- myaddr += written;
- ret += written;
- }
- return ret;
-}
-
static void
remote_fileio_func_open (char *buf)
{
CORE_ADDR ptrval;
- int length, retlength;
+ int length;
long num;
int flags, fd;
mode_t mode;
@@ -638,10 +621,9 @@ remote_fileio_func_open (char *buf)
}
mode = remote_fileio_mode_to_host (num, 1);
- /* Request pathname using 'm' packet. */
+ /* Request pathname. */
pathname = alloca (length);
- retlength = remote_read_bytes (ptrval, (gdb_byte *) pathname, length);
- if (retlength != length)
+ if (target_read_memory (ptrval, (gdb_byte *) pathname, length) != 0)
{
remote_fileio_ioerror ();
return;
@@ -819,10 +801,9 @@ remote_fileio_func_read (char *buf)
if (ret > 0)
{
- retlength = remote_fileio_write_bytes (ptrval, buffer, ret);
- if (retlength != ret)
- ret = -1; /* errno has been set to EIO in
- remote_fileio_write_bytes(). */
+ errno = target_write_memory (ptrval, buffer, ret);
+ if (errno != 0)
+ ret = -1;
}
if (ret < 0)
@@ -839,7 +820,7 @@ remote_fileio_func_write (char *buf)
long target_fd, num;
LONGEST lnum;
CORE_ADDR ptrval;
- int fd, ret, retlength;
+ int fd, ret;
gdb_byte *buffer;
size_t length;
@@ -871,8 +852,7 @@ remote_fileio_func_write (char *buf)
length = (size_t) num;
buffer = (gdb_byte *) xmalloc (length);
- retlength = remote_read_bytes (ptrval, buffer, length);
- if (retlength != length)
+ if (target_read_memory (ptrval, buffer, length) != 0)
{
xfree (buffer);
remote_fileio_ioerror ();
@@ -966,7 +946,7 @@ static void
remote_fileio_func_rename (char *buf)
{
CORE_ADDR old_ptr, new_ptr;
- int old_len, new_len, retlength;
+ int old_len, new_len;
char *oldpath, *newpath;
int ret, of, nf;
struct stat ost, nst;
@@ -987,8 +967,7 @@ remote_fileio_func_rename (char *buf)
/* Request oldpath using 'm' packet */
oldpath = alloca (old_len);
- retlength = remote_read_bytes (old_ptr, (gdb_byte *) oldpath, old_len);
- if (retlength != old_len)
+ if (target_read_memory (old_ptr, (gdb_byte *) oldpath, old_len) != 0)
{
remote_fileio_ioerror ();
return;
@@ -996,8 +975,7 @@ remote_fileio_func_rename (char *buf)
/* Request newpath using 'm' packet */
newpath = alloca (new_len);
- retlength = remote_read_bytes (new_ptr, (gdb_byte *) newpath, new_len);
- if (retlength != new_len)
+ if (target_read_memory (new_ptr, (gdb_byte *) newpath, new_len) != 0)
{
remote_fileio_ioerror ();
return;
@@ -1062,7 +1040,7 @@ static void
remote_fileio_func_unlink (char *buf)
{
CORE_ADDR ptrval;
- int length, retlength;
+ int length;
char *pathname;
int ret;
struct stat st;
@@ -1075,8 +1053,7 @@ remote_fileio_func_unlink (char *buf)
}
/* Request pathname using 'm' packet */
pathname = alloca (length);
- retlength = remote_read_bytes (ptrval, (gdb_byte *) pathname, length);
- if (retlength != length)
+ if (target_read_memory (ptrval, (gdb_byte *) pathname, length) != 0)
{
remote_fileio_ioerror ();
return;
@@ -1103,7 +1080,7 @@ static void
remote_fileio_func_stat (char *buf)
{
CORE_ADDR statptr, nameptr;
- int ret, namelength, retlength;
+ int ret, namelength;
char *pathname;
LONGEST lnum;
struct stat st;
@@ -1126,8 +1103,7 @@ remote_fileio_func_stat (char *buf)
/* Request pathname using 'm' packet */
pathname = alloca (namelength);
- retlength = remote_read_bytes (nameptr, (gdb_byte *) pathname, namelength);
- if (retlength != namelength)
+ if (target_read_memory (nameptr, (gdb_byte *) pathname, namelength) != 0)
{
remote_fileio_ioerror ();
return;
@@ -1151,10 +1127,9 @@ remote_fileio_func_stat (char *buf)
{
remote_fileio_to_fio_stat (&st, &fst);
remote_fileio_to_fio_uint (0, fst.fst_dev);
-
- retlength = remote_fileio_write_bytes (statptr,
- (gdb_byte *) &fst, sizeof fst);
- if (retlength != sizeof fst)
+
+ errno = target_write_memory (statptr, (gdb_byte *) &fst, sizeof fst);
+ if (errno != 0)
{
remote_fileio_return_errno (-1);
return;
@@ -1236,9 +1211,8 @@ remote_fileio_func_fstat (char *buf)
{
remote_fileio_to_fio_stat (&st, &fst);
- retlength = remote_fileio_write_bytes (ptrval, (gdb_byte *) &fst,
- sizeof fst);
- if (retlength != sizeof fst)
+ errno = target_write_memory (ptrval, (gdb_byte *) &fst, sizeof fst);
+ if (errno != 0)
{
remote_fileio_return_errno (-1);
return;
@@ -1289,9 +1263,8 @@ remote_fileio_func_gettimeofday (char *b
{
remote_fileio_to_fio_timeval (&tv, &ftv);
- retlength = remote_fileio_write_bytes (ptrval, (gdb_byte *) &ftv,
- sizeof ftv);
- if (retlength != sizeof ftv)
+ errno = target_write_memory (ptrval, (gdb_byte *) &ftv, sizeof ftv);
+ if (errno != 0)
{
remote_fileio_return_errno (-1);
return;
@@ -1336,8 +1309,7 @@ remote_fileio_func_system (char *buf)
{
/* Request commandline using 'm' packet */
cmdline = alloca (length);
- retlength = remote_read_bytes (ptrval, (gdb_byte *) cmdline, length);
- if (retlength != length)
+ if (target_read_memory (ptrval, (gdb_byte *) cmdline, length) != 0)
{
remote_fileio_ioerror ();
return;
Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c 2011-01-25 09:49:05.487639998 +0000
+++ src/gdb/remote.c 2011-01-25 11:40:49.007639996 +0000
@@ -6356,7 +6356,7 @@ remote_write_bytes_aux (const char *head
Returns number of bytes transferred, or 0 (setting errno) for
error. Only transfer a single packet. */
-int
+static int
remote_write_bytes (CORE_ADDR memaddr, const gdb_byte *myaddr, int len)
{
char *packet_format = 0;
@@ -6391,19 +6391,14 @@ remote_write_bytes (CORE_ADDR memaddr, c
Returns number of bytes transferred, or 0 for error. */
-/* NOTE: cagney/1999-10-18: This function (and its siblings in other
- remote targets) shouldn't attempt to read the entire buffer.
- Instead it should read a single packet worth of data and then
- return the byte size of that packet to the caller. The caller (its
- caller and its callers caller ;-) already contains code for
- handling partial reads. */
-
-int
+static int
remote_read_bytes (CORE_ADDR memaddr, gdb_byte *myaddr, int len)
{
struct remote_state *rs = get_remote_state ();
int max_buf_size; /* Max size of packet output buffer. */
- int origlen;
+ char *p;
+ int todo;
+ int i;
if (len <= 0)
return 0;
@@ -6412,56 +6407,37 @@ remote_read_bytes (CORE_ADDR memaddr, gd
/* The packet buffer will be large enough for the payload;
get_memory_packet_size ensures this. */
- origlen = len;
- while (len > 0)
+ /* Number if bytes that will fit. */
+ todo = min (len, max_buf_size / 2);
+
+ /* Construct "m"<memaddr>","<len>". */
+ memaddr = remote_address_masked (memaddr);
+ p = rs->buf;
+ *p++ = 'm';
+ p += hexnumstr (p, (ULONGEST) memaddr);
+ *p++ = ',';
+ p += hexnumstr (p, (ULONGEST) todo);
+ *p = '\0';
+ putpkt (rs->buf);
+ getpkt (&rs->buf, &rs->buf_size, 0);
+ if (rs->buf[0] == 'E'
+ && isxdigit (rs->buf[1]) && isxdigit (rs->buf[2])
+ && rs->buf[3] == '\0')
{
- char *p;
- int todo;
- int i;
-
- todo = min (len, max_buf_size / 2); /* num bytes that will fit. */
-
- /* construct "m"<memaddr>","<len>" */
- /* sprintf (rs->buf, "m%lx,%x", (unsigned long) memaddr, todo); */
- memaddr = remote_address_masked (memaddr);
- p = rs->buf;
- *p++ = 'm';
- p += hexnumstr (p, (ULONGEST) memaddr);
- *p++ = ',';
- p += hexnumstr (p, (ULONGEST) todo);
- *p = '\0';
-
- putpkt (rs->buf);
- getpkt (&rs->buf, &rs->buf_size, 0);
-
- if (rs->buf[0] == 'E'
- && isxdigit (rs->buf[1]) && isxdigit (rs->buf[2])
- && rs->buf[3] == '\0')
- {
- /* There is no correspondance between what the remote
- protocol uses for errors and errno codes. We would like
- a cleaner way of representing errors (big enough to
- include errno codes, bfd_error codes, and others). But
- for now just return EIO. */
- errno = EIO;
- return 0;
- }
-
- /* Reply describes memory byte by byte,
- each byte encoded as two hex characters. */
-
- p = rs->buf;
- if ((i = hex2bin (p, myaddr, todo)) < todo)
- {
- /* Reply is short. This means that we were able to read
- only part of what we wanted to. */
- return i + (origlen - len);
- }
- myaddr += todo;
- memaddr += todo;
- len -= todo;
+ /* There is no correspondance between what the remote protocol
+ uses for errors and errno codes. We would like a cleaner way
+ of representing errors (big enough to include errno codes,
+ bfd_error codes, and others). But for now just return
+ EIO. */
+ errno = EIO;
+ return 0;
}
- return origlen;
+ /* Reply describes memory byte by byte, each byte encoded as two hex
+ characters. */
+ p = rs->buf;
+ i = hex2bin (p, myaddr, todo);
+ /* Return what we have. Let higher layers handle partial reads. */
+ return i;
}
Index: src/gdb/remote.h
===================================================================
--- src.orig/gdb/remote.h 2011-01-24 12:42:54.126176998 +0000
+++ src/gdb/remote.h 2011-01-25 11:40:49.007639996 +0000
@@ -42,11 +42,6 @@ extern char *unpack_varlen_hex (char *bu
extern void async_remote_interrupt_twice (void *arg);
-extern int remote_write_bytes (CORE_ADDR memaddr, const gdb_byte *myaddr,
- int len);
-
-extern int remote_read_bytes (CORE_ADDR memaddr, gdb_byte *myaddr, int len);
-
void register_remote_g_packet_guess (struct gdbarch *gdbarch, int bytes,
const struct target_desc *tdesc);
void register_remote_support_xml (const char *);