public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFC 1/9] share "cell" code
  2014-01-20 19:18 [RFC 0/9] move more code into common/ Tom Tromey
@ 2014-01-20 19:18 ` Tom Tromey
  2014-02-05 20:05   ` Pedro Alves
  2014-01-20 19:18 ` [RFC 3/9] don't let bin2hex call strlen Tom Tromey
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Tom Tromey @ 2014-01-20 19:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

The "cell"-based printing code, like phex, was duplicated in both gdb
and gdbserver.  This patch merges the two implementations into a new
file in common/.

2014-01-20  Tom Tromey  <tromey@redhat.com>

	* utils.c (NUMCELLS, CELLSIZE, get_cell, decimal2str, pulongest)
	(plongest, thirty_two, phex, phex_nz, octal2str, hex_string)
	(hex_string_custom, int_string, core_addr_to_string)
	(core_addr_to_string_nz, host_address_to_string): Move to
	common/cells.c.
	* common/cells.h: New file.
	* common/cells.c: New file
	* Makefile.in (SFILES): Add common/cells.c.
	(HFILES_NO_SRCDIR): Add common/cells.h.
	(COMMON_OBS): Add cells.o.
	(cells.o): New target.

2014-01-20  Tom Tromey  <tromey@redhat.com>

	* utils.c (NUMCELLS, CELLSIZE, get_cell, decimal2str, pulongest)
	(plongest, thirty_two, phex_nz): Remove.
	* Makefile.in (SFILES): Add common/cells.c.
	(OBS): Add cells.o.
	(cell-ipas.o): New target.
	(cells.o): New target.
	(IPA_OBJS): Add cells-ipa.o.
---
 gdb/ChangeLog             |  14 ++
 gdb/Makefile.in           |  12 +-
 gdb/common/cells.c        | 332 ++++++++++++++++++++++++++++++++++++++++++++++
 gdb/common/cells.h        |  70 ++++++++++
 gdb/gdbserver/ChangeLog   |  10 ++
 gdb/gdbserver/Makefile.in |  12 +-
 gdb/gdbserver/utils.c     | 117 ----------------
 gdb/utils.c               | 296 -----------------------------------------
 8 files changed, 444 insertions(+), 419 deletions(-)
 create mode 100644 gdb/common/cells.c
 create mode 100644 gdb/common/cells.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 0fbaaa3..43ff30e 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -780,7 +780,7 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
 	common/gdb_vecs.c common/common-utils.c common/xml-utils.c \
 	common/ptid.c common/buffer.c gdb-dlfcn.c common/agent.c \
 	common/format.c common/filestuff.c btrace.c record-btrace.c ctf.c \
-	target/waitstatus.c
+	target/waitstatus.c common/cells.c
 
 LINTFILES = $(SFILES) $(YYFILES) $(CONFIG_SRCS) init.c
 
@@ -861,7 +861,8 @@ common/format.h common/host-defs.h utils.h common/queue.h \
 common/linux-osdata.h gdb-dlfcn.h auto-load.h probe.h stap-probe.h \
 gdb_bfd.h sparc-ravenscar-thread.h ppc-ravenscar-thread.h common/linux-btrace.h \
 ctf.h common/i386-cpuid.h common/i386-gcc-cpuid.h target/resume.h \
-target/wait.h target/waitstatus.h nat/linux-nat.h nat/linux-waitpid.h
+target/wait.h target/waitstatus.h nat/linux-nat.h nat/linux-waitpid.h \
+common/cells.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
@@ -958,7 +959,8 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	inferior.o osdata.o gdb_usleep.o record.o record-full.o gcore.o \
 	gdb_vecs.o jit.o progspace.o skip.o probe.o \
 	common-utils.o buffer.o ptid.o gdb-dlfcn.o common-agent.o \
-	format.o registry.o btrace.o record-btrace.o waitstatus.o
+	format.o registry.o btrace.o record-btrace.o waitstatus.o \
+	cells.o
 
 TSOBS = inflow.o
 
@@ -2070,6 +2072,10 @@ mips-linux-watch.o: ${srcdir}/common/mips-linux-watch.c
 	$(COMPILE) $(srcdir)/common/mips-linux-watch.c
 	$(POSTCOMPILE)
 
+cells.o: ${srcdir}/common/cells.c
+	$(COMPILE) $(srcdir)/common/cells.c
+	$(POSTCOMPILE)
+
 #
 # gdb/target/ dependencies
 #
diff --git a/gdb/common/cells.c b/gdb/common/cells.c
new file mode 100644
index 0000000..4ddad1c
--- /dev/null
+++ b/gdb/common/cells.c
@@ -0,0 +1,332 @@
+/* Cell-based print utility routines for GDB, the GNU debugger.
+
+   Copyright (C) 1986-2014 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifdef GDBSERVER
+#include "server.h"
+#else
+#include "defs.h"
+#endif
+
+#include <string.h>
+#include <stdint.h>
+
+/* Temporary storage using circular buffer.  */
+
+#define NUMCELLS 16
+#define CELLSIZE 50
+
+/* Return the next entry in the circular buffer.  */
+
+static char *
+get_cell (void)
+{
+  static char buf[NUMCELLS][CELLSIZE];
+  static int cell = 0;
+
+  if (++cell >= NUMCELLS)
+    cell = 0;
+  return buf[cell];
+}
+
+static char *
+decimal2str (char *sign, ULONGEST addr, int width)
+{
+  /* Steal code from valprint.c:print_decimal().  Should this worry
+     about the real size of addr as the above does?  */
+  unsigned long temp[3];
+  char *str = get_cell ();
+  int i = 0;
+
+  do
+    {
+      temp[i] = addr % (1000 * 1000 * 1000);
+      addr /= (1000 * 1000 * 1000);
+      i++;
+      width -= 9;
+    }
+  while (addr != 0 && i < (sizeof (temp) / sizeof (temp[0])));
+
+  width += 9;
+  if (width < 0)
+    width = 0;
+
+  switch (i)
+    {
+    case 1:
+      xsnprintf (str, CELLSIZE, "%s%0*lu", sign, width, temp[0]);
+      break;
+    case 2:
+      xsnprintf (str, CELLSIZE, "%s%0*lu%09lu", sign, width,
+		 temp[1], temp[0]);
+      break;
+    case 3:
+      xsnprintf (str, CELLSIZE, "%s%0*lu%09lu%09lu", sign, width,
+		 temp[2], temp[1], temp[0]);
+      break;
+    default:
+      internal_error (__FILE__, __LINE__,
+		      _("failed internal consistency check"));
+    }
+
+  return str;
+}
+
+static char *
+octal2str (ULONGEST addr, int width)
+{
+  unsigned long temp[3];
+  char *str = get_cell ();
+  int i = 0;
+
+  do
+    {
+      temp[i] = addr % (0100000 * 0100000);
+      addr /= (0100000 * 0100000);
+      i++;
+      width -= 10;
+    }
+  while (addr != 0 && i < (sizeof (temp) / sizeof (temp[0])));
+
+  width += 10;
+  if (width < 0)
+    width = 0;
+
+  switch (i)
+    {
+    case 1:
+      if (temp[0] == 0)
+	xsnprintf (str, CELLSIZE, "%*o", width, 0);
+      else
+	xsnprintf (str, CELLSIZE, "0%0*lo", width, temp[0]);
+      break;
+    case 2:
+      xsnprintf (str, CELLSIZE, "0%0*lo%010lo", width, temp[1], temp[0]);
+      break;
+    case 3:
+      xsnprintf (str, CELLSIZE, "0%0*lo%010lo%010lo", width,
+		 temp[2], temp[1], temp[0]);
+      break;
+    default:
+      internal_error (__FILE__, __LINE__,
+		      _("failed internal consistency check"));
+    }
+
+  return str;
+}
+
+/* See cells.h.  */
+
+char *
+pulongest (ULONGEST u)
+{
+  return decimal2str ("", u, 0);
+}
+
+/* See cells.h.  */
+
+char *
+plongest (LONGEST l)
+{
+  if (l < 0)
+    return decimal2str ("-", -l, 0);
+  else
+    return decimal2str ("", l, 0);
+}
+
+/* Eliminate warning from compiler on 32-bit systems.  */
+static int thirty_two = 32;
+
+/* See cells.h.  */
+
+char *
+phex (ULONGEST l, int sizeof_l)
+{
+  char *str;
+
+  switch (sizeof_l)
+    {
+    case 8:
+      str = get_cell ();
+      xsnprintf (str, CELLSIZE, "%08lx%08lx",
+		 (unsigned long) (l >> thirty_two),
+		 (unsigned long) (l & 0xffffffff));
+      break;
+    case 4:
+      str = get_cell ();
+      xsnprintf (str, CELLSIZE, "%08lx", (unsigned long) l);
+      break;
+    case 2:
+      str = get_cell ();
+      xsnprintf (str, CELLSIZE, "%04x", (unsigned short) (l & 0xffff));
+      break;
+    default:
+      str = phex (l, sizeof (l));
+      break;
+    }
+
+  return str;
+}
+
+/* See cells.h.  */
+
+char *
+phex_nz (ULONGEST l, int sizeof_l)
+{
+  char *str;
+
+  switch (sizeof_l)
+    {
+    case 8:
+      {
+	unsigned long high = (unsigned long) (l >> thirty_two);
+
+	str = get_cell ();
+	if (high == 0)
+	  xsnprintf (str, CELLSIZE, "%lx",
+		     (unsigned long) (l & 0xffffffff));
+	else
+	  xsnprintf (str, CELLSIZE, "%lx%08lx", high,
+		     (unsigned long) (l & 0xffffffff));
+	break;
+      }
+    case 4:
+      str = get_cell ();
+      xsnprintf (str, CELLSIZE, "%lx", (unsigned long) l);
+      break;
+    case 2:
+      str = get_cell ();
+      xsnprintf (str, CELLSIZE, "%x", (unsigned short) (l & 0xffff));
+      break;
+    default:
+      str = phex_nz (l, sizeof (l));
+      break;
+    }
+
+  return str;
+}
+
+/* See cells.h.  */
+
+char *
+hex_string (LONGEST num)
+{
+  char *result = get_cell ();
+
+  xsnprintf (result, CELLSIZE, "0x%s", phex_nz (num, sizeof (num)));
+  return result;
+}
+
+/* See cells.h.  */
+
+char *
+hex_string_custom (LONGEST num, int width)
+{
+  char *result = get_cell ();
+  char *result_end = result + CELLSIZE - 1;
+  const char *hex = phex_nz (num, sizeof (num));
+  int hex_len = strlen (hex);
+
+  if (hex_len > width)
+    width = hex_len;
+  if (width + 2 >= CELLSIZE)
+    internal_error (__FILE__, __LINE__, _("\
+hex_string_custom: insufficient space to store result"));
+
+  strcpy (result_end - width - 2, "0x");
+  memset (result_end - width, '0', width);
+  strcpy (result_end - hex_len, hex);
+  return result_end - width - 2;
+}
+
+/* See cells.h.  */
+
+char *
+int_string (LONGEST val, int radix, int is_signed, int width, 
+	    int use_c_format)
+{
+  switch (radix) 
+    {
+    case 16:
+      {
+	char *result;
+
+	if (width == 0)
+	  result = hex_string (val);
+	else
+	  result = hex_string_custom (val, width);
+	if (! use_c_format)
+	  result += 2;
+	return result;
+      }
+    case 10:
+      {
+	if (is_signed && val < 0)
+	  return decimal2str ("-", -val, width);
+	else
+	  return decimal2str ("", val, width);
+      }
+    case 8:
+      {
+	char *result = octal2str (val, width);
+
+	if (use_c_format || val == 0)
+	  return result;
+	else
+	  return result + 1;
+      }
+    default:
+      internal_error (__FILE__, __LINE__,
+		      _("failed internal consistency check"));
+    }
+}	
+
+/* See cells.h.  */
+
+const char *
+core_addr_to_string (const CORE_ADDR addr)
+{
+  char *str = get_cell ();
+
+  strcpy (str, "0x");
+  strcat (str, phex (addr, sizeof (addr)));
+  return str;
+}
+
+/* See cells.h.  */
+
+const char *
+core_addr_to_string_nz (const CORE_ADDR addr)
+{
+  char *str = get_cell ();
+
+  strcpy (str, "0x");
+  strcat (str, phex_nz (addr, sizeof (addr)));
+  return str;
+}
+
+/* See cells.h.  */
+
+const char *
+host_address_to_string (const void *addr)
+{
+  char *str = get_cell ();
+
+  xsnprintf (str, CELLSIZE, "0x%s", phex_nz ((uintptr_t) addr, sizeof (addr)));
+  return str;
+}
diff --git a/gdb/common/cells.h b/gdb/common/cells.h
new file mode 100644
index 0000000..35b9739
--- /dev/null
+++ b/gdb/common/cells.h
@@ -0,0 +1,70 @@
+/* Cell-based print utility routines for GDB, the GNU debugger.
+
+   Copyright (C) 1986-2014 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef COMMON_CELLS_H
+#define COMMON_CELLS_H
+
+/* %d for LONGEST.  The result is stored in a circular static buffer,
+   NUMCELLS deep.  */
+
+extern char *pulongest (ULONGEST u);
+
+/* %u for ULONGEST.  The result is stored in a circular static buffer,
+   NUMCELLS deep.  */
+
+extern char *plongest (LONGEST l);
+
+extern char *phex (ULONGEST l, int sizeof_l);
+
+/* Convert a ULONGEST into a HEX string, like %lx.  The result is
+   stored in a circular static buffer, NUMCELLS deep.  */
+
+extern char *phex_nz (ULONGEST l, int sizeof_l);
+
+/* Converts a LONGEST to a C-format hexadecimal literal and stores it
+   in a static string.  Returns a pointer to this string.  */
+
+extern char *hex_string (LONGEST num);
+
+/* Converts a LONGEST number to a C-format hexadecimal literal and
+   stores it in a static string.  Returns a pointer to this string
+   that is valid until the next call.  The number is padded on the
+   left with 0s to at least WIDTH characters.  */
+
+extern char *hex_string_custom (LONGEST num, int width);
+
+/* Convert VAL to a numeral in the given radix.  For
+ * radix 10, IS_SIGNED may be true, indicating a signed quantity;
+ * otherwise VAL is interpreted as unsigned.  If WIDTH is supplied, 
+ * it is the minimum width (0-padded if needed).  USE_C_FORMAT means
+ * to use C format in all cases.  If it is false, then 'x' 
+ * and 'o' formats do not include a prefix (0x or leading 0).  */
+
+extern char *int_string (LONGEST val, int radix, int is_signed, int width, 
+			 int use_c_format);	
+
+/* Convert a CORE_ADDR into a string.  */
+
+extern const char *core_addr_to_string (const CORE_ADDR addr);
+
+extern const char *core_addr_to_string_nz (const CORE_ADDR addr);
+
+extern const char *host_address_to_string (const void *addr);
+
+#endif /* COMMON_CELLS_H */
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index e72ee6b..60d6e2f 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -163,7 +163,7 @@ SFILES=	$(srcdir)/gdbreplay.c $(srcdir)/inferiors.c $(srcdir)/dll.c \
 	$(srcdir)/common/linux-osdata.c $(srcdir)/common/ptid.c \
 	$(srcdir)/common/buffer.c $(srcdir)/common/linux-btrace.c \
 	$(srcdir)/common/filestuff.c $(srcdir)/target/waitstatus.c \
-    $(srcdir)/common/mips-linux-watch.c
+    $(srcdir)/common/mips-linux-watch.c $(srcdir)/common/cells.c
 
 DEPFILES = @GDBSERVER_DEPFILES@
 
@@ -176,7 +176,7 @@ OBS = agent.o ax.o inferiors.o regcache.o remote-utils.o server.o signals.o \
       target.o waitstatus.o utils.o version.o vec.o gdb_vecs.o \
       mem-break.o hostio.o event-loop.o tracepoint.o xml-utils.o \
       common-utils.o ptid.o buffer.o format.o filestuff.o dll.o notif.o \
-      tdesc.o $(XML_BUILTIN) $(DEPFILES) $(LIBOBJS)
+      tdesc.o cells.o $(XML_BUILTIN) $(DEPFILES) $(LIBOBJS)
 GDBREPLAY_OBS = gdbreplay.o version.o
 GDBSERVER_LIBS = @GDBSERVER_LIBS@
 XM_CLIBS = @LIBS@
@@ -291,7 +291,7 @@ gdbreplay$(EXEEXT): $(GDBREPLAY_OBS) $(LIBGNU)
 	${CC-LD} $(INTERNAL_CFLAGS) $(INTERNAL_LDFLAGS) -o gdbreplay$(EXEEXT) $(GDBREPLAY_OBS) \
 	  $(XM_CLIBS) $(LIBGNU)
 
-IPA_OBJS=ax-ipa.o tracepoint-ipa.o format-ipa.o utils-ipa.o regcache-ipa.o remote-utils-ipa.o common-utils-ipa.o tdesc-ipa.o ${IPA_DEPFILES}
+IPA_OBJS=ax-ipa.o tracepoint-ipa.o format-ipa.o utils-ipa.o regcache-ipa.o remote-utils-ipa.o common-utils-ipa.o tdesc-ipa.o cells-ipa.o ${IPA_DEPFILES}
 
 IPA_LIB=libinproctrace.so
 
@@ -471,6 +471,9 @@ amd64-linux-ipa.o: amd64-linux.c
 tdesc-ipa.o: tdesc.c
 	$(IPAGENT_COMPILE) $<
 	$(POSTCOMPILE)
+cells-ipa.o: ../common/cells.c
+	$(IPAGENT_COMPILE) $<
+	$(POSTCOMPILE)
 
 ax.o: ax.c
 	$(COMPILE) $(WARN_CFLAGS_NO_FORMAT) $<
@@ -479,6 +482,9 @@ ax.o: ax.c
 signals.o: ../common/signals.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
+cells.o: ../common/cells.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
 linux-procfs.o: ../common/linux-procfs.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
diff --git a/gdb/gdbserver/utils.c b/gdb/gdbserver/utils.c
index eff4499..19955dc 100644
--- a/gdb/gdbserver/utils.c
+++ b/gdb/gdbserver/utils.c
@@ -165,123 +165,6 @@ internal_error (const char *file, int line, const char *fmt, ...)
   exit (1);
 }
 
-/* Temporary storage using circular buffer.  */
-#define NUMCELLS 10
-#define CELLSIZE 50
-
-/* Return the next entry in the circular buffer.  */
-
-static char *
-get_cell (void)
-{
-  static char buf[NUMCELLS][CELLSIZE];
-  static int cell = 0;
-  if (++cell >= NUMCELLS)
-    cell = 0;
-  return buf[cell];
-}
-
-static char *
-decimal2str (char *sign, ULONGEST addr)
-{
-  /* Steal code from valprint.c:print_decimal().  Should this worry
-     about the real size of addr as the above does? */
-  unsigned long temp[3];
-  char *str = get_cell ();
-  int i = 0;
-  int width = 9;
-
-  do
-    {
-      temp[i] = addr % (1000 * 1000 * 1000);
-      addr /= (1000 * 1000 * 1000);
-      i++;
-    }
-  while (addr != 0 && i < (sizeof (temp) / sizeof (temp[0])));
-
-  switch (i)
-    {
-    case 1:
-      xsnprintf (str, CELLSIZE, "%s%0*lu", sign, width, temp[0]);
-      break;
-    case 2:
-      xsnprintf (str, CELLSIZE, "%s%0*lu%09lu", sign, width,
-		 temp[1], temp[0]);
-      break;
-    case 3:
-      xsnprintf (str, CELLSIZE, "%s%0*lu%09lu%09lu", sign, width,
-		 temp[2], temp[1], temp[0]);
-      break;
-    default:
-      internal_error (__FILE__, __LINE__,
-		      "failed internal consistency check");
-    }
-
-  return str;
-}
-
-/* %u for ULONGEST.  The result is stored in a circular static buffer,
-   NUMCELLS deep.  */
-
-char *
-pulongest (ULONGEST u)
-{
-  return decimal2str ("", u);
-}
-
-/* %d for LONGEST.  The result is stored in a circular static buffer,
-   NUMCELLS deep.  */
-
-char *
-plongest (LONGEST l)
-{
-  if (l < 0)
-    return decimal2str ("-", -l);
-  else
-    return decimal2str ("", l);
-}
-
-/* Eliminate warning from compiler on 32-bit systems.  */
-static int thirty_two = 32;
-
-/* Convert a ULONGEST into a HEX string, like %lx.  The result is
-   stored in a circular static buffer, NUMCELLS deep.  */
-
-char *
-phex_nz (ULONGEST l, int sizeof_l)
-{
-  char *str;
-
-  switch (sizeof_l)
-    {
-    case 8:
-      {
-	unsigned long high = (unsigned long) (l >> thirty_two);
-	str = get_cell ();
-	if (high == 0)
-	  xsnprintf (str, CELLSIZE, "%lx",
-		     (unsigned long) (l & 0xffffffff));
-	else
-	  xsnprintf (str, CELLSIZE, "%lx%08lx", high,
-		     (unsigned long) (l & 0xffffffff));
-	break;
-      }
-    case 4:
-      str = get_cell ();
-      xsnprintf (str, CELLSIZE, "%lx", (unsigned long) l);
-      break;
-    case 2:
-      str = get_cell ();
-      xsnprintf (str, CELLSIZE, "%x", (unsigned short) (l & 0xffff));
-      break;
-    default:
-      str = phex_nz (l, sizeof (l));
-      break;
-    }
-
-  return str;
-}
-
 /* Convert a CORE_ADDR into a HEX string, like %lx.
    The result is stored in a circular static buffer, NUMCELLS deep.  */
 
diff --git a/gdb/utils.c b/gdb/utils.c
index e062e89..2506c9c 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -2748,21 +2748,6 @@ When set, debugging messages will be marked with seconds and microseconds."),
 			   &setdebuglist, &showdebuglist);
 }
 
-/* Print routines to handle variable size regs, etc.  */
-/* Temporary storage using circular buffer.  */
-#define NUMCELLS 16
-#define CELLSIZE 50
-static char *
-get_cell (void)
-{
-  static char buf[NUMCELLS][CELLSIZE];
-  static int cell = 0;
-
-  if (++cell >= NUMCELLS)
-    cell = 0;
-  return buf[cell];
-}
-
 const char *
 paddress (struct gdbarch *gdbarch, CORE_ADDR addr)
 {
@@ -2822,278 +2807,6 @@ core_addr_eq (const void *ap, const void *bp)
   return *addr_ap == *addr_bp;
 }
 
-static char *
-decimal2str (char *sign, ULONGEST addr, int width)
-{
-  /* Steal code from valprint.c:print_decimal().  Should this worry
-     about the real size of addr as the above does?  */
-  unsigned long temp[3];
-  char *str = get_cell ();
-  int i = 0;
-
-  do
-    {
-      temp[i] = addr % (1000 * 1000 * 1000);
-      addr /= (1000 * 1000 * 1000);
-      i++;
-      width -= 9;
-    }
-  while (addr != 0 && i < (sizeof (temp) / sizeof (temp[0])));
-
-  width += 9;
-  if (width < 0)
-    width = 0;
-
-  switch (i)
-    {
-    case 1:
-      xsnprintf (str, CELLSIZE, "%s%0*lu", sign, width, temp[0]);
-      break;
-    case 2:
-      xsnprintf (str, CELLSIZE, "%s%0*lu%09lu", sign, width,
-		 temp[1], temp[0]);
-      break;
-    case 3:
-      xsnprintf (str, CELLSIZE, "%s%0*lu%09lu%09lu", sign, width,
-		 temp[2], temp[1], temp[0]);
-      break;
-    default:
-      internal_error (__FILE__, __LINE__,
-		      _("failed internal consistency check"));
-    }
-
-  return str;
-}
-
-static char *
-octal2str (ULONGEST addr, int width)
-{
-  unsigned long temp[3];
-  char *str = get_cell ();
-  int i = 0;
-
-  do
-    {
-      temp[i] = addr % (0100000 * 0100000);
-      addr /= (0100000 * 0100000);
-      i++;
-      width -= 10;
-    }
-  while (addr != 0 && i < (sizeof (temp) / sizeof (temp[0])));
-
-  width += 10;
-  if (width < 0)
-    width = 0;
-
-  switch (i)
-    {
-    case 1:
-      if (temp[0] == 0)
-	xsnprintf (str, CELLSIZE, "%*o", width, 0);
-      else
-	xsnprintf (str, CELLSIZE, "0%0*lo", width, temp[0]);
-      break;
-    case 2:
-      xsnprintf (str, CELLSIZE, "0%0*lo%010lo", width, temp[1], temp[0]);
-      break;
-    case 3:
-      xsnprintf (str, CELLSIZE, "0%0*lo%010lo%010lo", width,
-		 temp[2], temp[1], temp[0]);
-      break;
-    default:
-      internal_error (__FILE__, __LINE__,
-		      _("failed internal consistency check"));
-    }
-
-  return str;
-}
-
-char *
-pulongest (ULONGEST u)
-{
-  return decimal2str ("", u, 0);
-}
-
-char *
-plongest (LONGEST l)
-{
-  if (l < 0)
-    return decimal2str ("-", -l, 0);
-  else
-    return decimal2str ("", l, 0);
-}
-
-/* Eliminate warning from compiler on 32-bit systems.  */
-static int thirty_two = 32;
-
-char *
-phex (ULONGEST l, int sizeof_l)
-{
-  char *str;
-
-  switch (sizeof_l)
-    {
-    case 8:
-      str = get_cell ();
-      xsnprintf (str, CELLSIZE, "%08lx%08lx",
-		 (unsigned long) (l >> thirty_two),
-		 (unsigned long) (l & 0xffffffff));
-      break;
-    case 4:
-      str = get_cell ();
-      xsnprintf (str, CELLSIZE, "%08lx", (unsigned long) l);
-      break;
-    case 2:
-      str = get_cell ();
-      xsnprintf (str, CELLSIZE, "%04x", (unsigned short) (l & 0xffff));
-      break;
-    default:
-      str = phex (l, sizeof (l));
-      break;
-    }
-
-  return str;
-}
-
-char *
-phex_nz (ULONGEST l, int sizeof_l)
-{
-  char *str;
-
-  switch (sizeof_l)
-    {
-    case 8:
-      {
-	unsigned long high = (unsigned long) (l >> thirty_two);
-
-	str = get_cell ();
-	if (high == 0)
-	  xsnprintf (str, CELLSIZE, "%lx",
-		     (unsigned long) (l & 0xffffffff));
-	else
-	  xsnprintf (str, CELLSIZE, "%lx%08lx", high,
-		     (unsigned long) (l & 0xffffffff));
-	break;
-      }
-    case 4:
-      str = get_cell ();
-      xsnprintf (str, CELLSIZE, "%lx", (unsigned long) l);
-      break;
-    case 2:
-      str = get_cell ();
-      xsnprintf (str, CELLSIZE, "%x", (unsigned short) (l & 0xffff));
-      break;
-    default:
-      str = phex_nz (l, sizeof (l));
-      break;
-    }
-
-  return str;
-}
-
-/* Converts a LONGEST to a C-format hexadecimal literal and stores it
-   in a static string.  Returns a pointer to this string.  */
-char *
-hex_string (LONGEST num)
-{
-  char *result = get_cell ();
-
-  xsnprintf (result, CELLSIZE, "0x%s", phex_nz (num, sizeof (num)));
-  return result;
-}
-
-/* Converts a LONGEST number to a C-format hexadecimal literal and
-   stores it in a static string.  Returns a pointer to this string
-   that is valid until the next call.  The number is padded on the
-   left with 0s to at least WIDTH characters.  */
-char *
-hex_string_custom (LONGEST num, int width)
-{
-  char *result = get_cell ();
-  char *result_end = result + CELLSIZE - 1;
-  const char *hex = phex_nz (num, sizeof (num));
-  int hex_len = strlen (hex);
-
-  if (hex_len > width)
-    width = hex_len;
-  if (width + 2 >= CELLSIZE)
-    internal_error (__FILE__, __LINE__, _("\
-hex_string_custom: insufficient space to store result"));
-
-  strcpy (result_end - width - 2, "0x");
-  memset (result_end - width, '0', width);
-  strcpy (result_end - hex_len, hex);
-  return result_end - width - 2;
-}
-
-/* Convert VAL to a numeral in the given radix.  For
- * radix 10, IS_SIGNED may be true, indicating a signed quantity;
- * otherwise VAL is interpreted as unsigned.  If WIDTH is supplied, 
- * it is the minimum width (0-padded if needed).  USE_C_FORMAT means
- * to use C format in all cases.  If it is false, then 'x' 
- * and 'o' formats do not include a prefix (0x or leading 0).  */
-
-char *
-int_string (LONGEST val, int radix, int is_signed, int width, 
-	    int use_c_format)
-{
-  switch (radix) 
-    {
-    case 16:
-      {
-	char *result;
-
-	if (width == 0)
-	  result = hex_string (val);
-	else
-	  result = hex_string_custom (val, width);
-	if (! use_c_format)
-	  result += 2;
-	return result;
-      }
-    case 10:
-      {
-	if (is_signed && val < 0)
-	  return decimal2str ("-", -val, width);
-	else
-	  return decimal2str ("", val, width);
-      }
-    case 8:
-      {
-	char *result = octal2str (val, width);
-
-	if (use_c_format || val == 0)
-	  return result;
-	else
-	  return result + 1;
-      }
-    default:
-      internal_error (__FILE__, __LINE__,
-		      _("failed internal consistency check"));
-    }
-}	
-
-/* Convert a CORE_ADDR into a string.  */
-const char *
-core_addr_to_string (const CORE_ADDR addr)
-{
-  char *str = get_cell ();
-
-  strcpy (str, "0x");
-  strcat (str, phex (addr, sizeof (addr)));
-  return str;
-}
-
-const char *
-core_addr_to_string_nz (const CORE_ADDR addr)
-{
-  char *str = get_cell ();
-
-  strcpy (str, "0x");
-  strcat (str, phex_nz (addr, sizeof (addr)));
-  return str;
-}
-
 /* Convert a string back into a CORE_ADDR.  */
 CORE_ADDR
 string_to_core_addr (const char *my_string)
@@ -3132,15 +2845,6 @@ string_to_core_addr (const char *my_string)
   return addr;
 }
 
-const char *
-host_address_to_string (const void *addr)
-{
-  char *str = get_cell ();
-
-  xsnprintf (str, CELLSIZE, "0x%s", phex_nz ((uintptr_t) addr, sizeof (addr)));
-  return str;
-}
-
 char *
 gdb_realpath (const char *filename)
 {
-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC 6/9] replace convert_int_to_ascii with bin2hex
  2014-01-20 19:18 [RFC 0/9] move more code into common/ Tom Tromey
  2014-01-20 19:18 ` [RFC 1/9] share "cell" code Tom Tromey
  2014-01-20 19:18 ` [RFC 3/9] don't let bin2hex call strlen Tom Tromey
@ 2014-01-20 19:18 ` Tom Tromey
  2014-02-05 20:39   ` Pedro Alves
  2014-01-20 19:18 ` [RFC 2/9] move some rsp bits into rsp-low.h Tom Tromey
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Tom Tromey @ 2014-01-20 19:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

convert_int_to_ascii is identical to bin2hex.  This removes the
former.  In this case I made the choice of which to keep on the basis
that I consider the name bin2hex to be superior to
convert_int_to_ascii.

2014-01-20  Tom Tromey  <tromey@redhat.com>

	* common/rsp-low.h (convert_int_to_ascii): Don't declare.
	* common/rsp-low.c (convert_int_to_ascii): Remove.

2014-01-20  Tom Tromey  <tromey@redhat.com>

	* ax.c (gdb_unparse_agent_expr): Use bin2hex, not
	convert_int_to_ascii.
	* regcache.c (registers_to_string, collect_register_as_string):
	Likewise.
	* remote-utils.c (look_up_one_symbol, relocate_instruction):
	Likewise.
	* server.c (process_serial_event): Likewise.
	* tracepoint.c (cmd_qtstatus, response_source, response_tsv)
	(cmd_qtbuffer, cstr_to_hexstr): Likewise.
---
 gdb/ChangeLog                |  5 +++++
 gdb/common/rsp-low.c         | 16 ----------------
 gdb/common/rsp-low.h         |  2 --
 gdb/gdbserver/ChangeLog      | 12 ++++++++++++
 gdb/gdbserver/ax.c           |  2 +-
 gdb/gdbserver/regcache.c     |  7 +++----
 gdb/gdbserver/remote-utils.c |  4 ++--
 gdb/gdbserver/server.c       |  2 +-
 gdb/gdbserver/tracepoint.c   | 10 +++++-----
 9 files changed, 29 insertions(+), 31 deletions(-)

diff --git a/gdb/common/rsp-low.c b/gdb/common/rsp-low.c
index e288c6a..ea044ee 100644
--- a/gdb/common/rsp-low.c
+++ b/gdb/common/rsp-low.c
@@ -172,22 +172,6 @@ bin2hex (const gdb_byte *bin, char *hex, int count)
   return i;
 }
 
-void
-convert_int_to_ascii (const unsigned char *from, char *to, int n)
-{
-  int nib;
-  int ch;
-  while (n--)
-    {
-      ch = *from++;
-      nib = ((ch & 0xf0) >> 4) & 0x0f;
-      *to++ = tohex (nib);
-      nib = ch & 0x0f;
-      *to++ = tohex (nib);
-    }
-  *to++ = 0;
-}
-
 int
 remote_escape_output (const gdb_byte *buffer, int len,
 		      gdb_byte *out_buf, int *out_len,
diff --git a/gdb/common/rsp-low.h b/gdb/common/rsp-low.h
index 4903cd8..8c70dee 100644
--- a/gdb/common/rsp-low.h
+++ b/gdb/common/rsp-low.h
@@ -42,8 +42,6 @@ extern void convert_ascii_to_int (const char *from, unsigned char *to, int n);
 
 extern int bin2hex (const gdb_byte *bin, char *hex, int count);
 
-extern void convert_int_to_ascii (const unsigned char *from, char *to, int n);
-
 /* Convert BUFFER, binary data at least LEN bytes long, into escaped
    binary data in OUT_BUF.  Set *OUT_LEN to the length of the data
    encoded in OUT_BUF, and return the number of bytes in OUT_BUF
diff --git a/gdb/gdbserver/ax.c b/gdb/gdbserver/ax.c
index f62a613..1eefaf3 100644
--- a/gdb/gdbserver/ax.c
+++ b/gdb/gdbserver/ax.c
@@ -120,7 +120,7 @@ gdb_unparse_agent_expr (struct agent_expr *aexpr)
   char *rslt;
 
   rslt = xmalloc (2 * aexpr->length + 1);
-  convert_int_to_ascii (aexpr->bytes, rslt, aexpr->length);
+  bin2hex (aexpr->bytes, rslt, aexpr->length);
   return rslt;
 }
 
diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c
index f51e498..6e5aa08 100644
--- a/gdb/gdbserver/regcache.c
+++ b/gdb/gdbserver/regcache.c
@@ -207,8 +207,7 @@ registers_to_string (struct regcache *regcache, char *buf)
     {
       if (regcache->register_status[i] == REG_VALID)
 	{
-	  convert_int_to_ascii (registers, buf,
-				register_size (tdesc, i));
+	  bin2hex (registers, buf, register_size (tdesc, i));
 	  buf += register_size (tdesc, i) * 2;
 	}
       else
@@ -419,8 +418,8 @@ collect_register (struct regcache *regcache, int n, void *buf)
 void
 collect_register_as_string (struct regcache *regcache, int n, char *buf)
 {
-  convert_int_to_ascii (register_data (regcache, n, 1), buf,
-			register_size (regcache->tdesc, n));
+  bin2hex (register_data (regcache, n, 1), buf,
+	   register_size (regcache->tdesc, n));
 }
 
 void
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 9d0ec55..a10af64 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -1423,7 +1423,7 @@ look_up_one_symbol (const char *name, CORE_ADDR *addrp, int may_ask_gdb)
       decode_m_packet (&own_buf[1], &mem_addr, &mem_len);
       mem_buf = xmalloc (mem_len);
       if (read_inferior_memory (mem_addr, mem_buf, mem_len) == 0)
-	convert_int_to_ascii (mem_buf, own_buf, mem_len);
+	bin2hex (mem_buf, own_buf, mem_len);
       else
 	write_enn (own_buf);
       free (mem_buf);
@@ -1507,7 +1507,7 @@ relocate_instruction (CORE_ADDR *to, CORE_ADDR oldloc)
 	  decode_m_packet (&own_buf[1], &mem_addr, &mem_len);
 	  mem_buf = xmalloc (mem_len);
 	  if (read_inferior_memory (mem_addr, mem_buf, mem_len) == 0)
-	    convert_int_to_ascii (mem_buf, own_buf, mem_len);
+	    bin2hex (mem_buf, own_buf, mem_len);
 	  else
 	    write_enn (own_buf);
 	}
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 2fe81cf..a53d9a2 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -3487,7 +3487,7 @@ process_serial_event (void)
       if (res < 0)
 	write_enn (own_buf);
       else
-	convert_int_to_ascii (mem_buf, own_buf, res);
+	bin2hex (mem_buf, own_buf, res);
       break;
     case 'M':
       require_running (own_buf);
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 8dffe83..8be12ed 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -3648,7 +3648,7 @@ cmd_qtstatus (char *packet)
       p = stop_reason_rsp = alloca (strlen ("terror:") + hexstr_len + 1);
       strcpy (p, "terror:");
       p += strlen (p);
-      convert_int_to_ascii ((gdb_byte *) result_name, p, strlen (result_name));
+      bin2hex ((gdb_byte *) result_name, p, strlen (result_name));
     }
 
   /* If this was a forced stop, include any stop note that was supplied.  */
@@ -3767,7 +3767,7 @@ response_source (char *packet,
 
   len = strlen (src->str);
   buf = alloca (len * 2 + 1);
-  convert_int_to_ascii ((gdb_byte *) src->str, buf, len);
+  bin2hex ((gdb_byte *) src->str, buf, len);
 
   sprintf (packet, "Z%x:%s:%s:%x:%x:%s",
 	   tpoint->number, paddress (tpoint->address),
@@ -3856,7 +3856,7 @@ response_tsv (char *packet, struct trace_state_variable *tsv)
     {
       namelen = strlen (tsv->name);
       buf = alloca (namelen * 2 + 1);
-      convert_int_to_ascii ((gdb_byte *) tsv->name, buf, namelen);
+      bin2hex ((gdb_byte *) tsv->name, buf, namelen);
     }
 
   sprintf (packet, "%x:%s:%x:%s", tsv->number, phex_nz (tsv->initial_value, 0),
@@ -4026,7 +4026,7 @@ cmd_qtbuffer (char *own_buf)
   if (num >= (PBUFSIZ - 16) / 2 )
     num = (PBUFSIZ - 16) / 2;
 
-  convert_int_to_ascii (tbp, own_buf, num);
+  bin2hex (tbp, own_buf, num);
 }
 
 static void
@@ -6915,7 +6915,7 @@ cstr_to_hexstr (const char *str)
 {
   int len = strlen (str);
   char *hexstr = xmalloc (len * 2 + 1);
-  convert_int_to_ascii ((gdb_byte *) str, hexstr, len);
+  bin2hex ((gdb_byte *) str, hexstr, len);
   return hexstr;
 }
 
-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC 3/9] don't let bin2hex call strlen
  2014-01-20 19:18 [RFC 0/9] move more code into common/ Tom Tromey
  2014-01-20 19:18 ` [RFC 1/9] share "cell" code Tom Tromey
@ 2014-01-20 19:18 ` Tom Tromey
  2014-02-05 20:36   ` Pedro Alves
  2014-01-20 19:18 ` [RFC 6/9] replace convert_int_to_ascii with bin2hex Tom Tromey
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Tom Tromey @ 2014-01-20 19:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

Currently bin2hex may call strlen if the length argument is zero.
This prevents some function unification; and also it seems cleaner to
me not to have a special meaning for a zero length.

2014-01-20  Tom Tromey  <tromey@redhat.com>

	* common/rsp-low.c (bin2hex): Never take strlen of argument.
	* remote.c (extended_remote_run, remote_rcmd)
	(remote_download_trace_state_variable, remote_save_trace_data)
	(remote_set_trace_notes): Update.
	* tracepoint.c (encode_source_string, tfile_write_status)
	(tfile_write_uploaded_tsv): Update.
---
 gdb/ChangeLog        |  9 +++++++++
 gdb/common/rsp-low.c |  4 ----
 gdb/remote.c         | 18 ++++++++++--------
 gdb/tracepoint.c     | 10 +++++-----
 4 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/gdb/common/rsp-low.c b/gdb/common/rsp-low.c
index 0e3da39..9c5119b 100644
--- a/gdb/common/rsp-low.c
+++ b/gdb/common/rsp-low.c
@@ -163,10 +163,6 @@ bin2hex (const gdb_byte *bin, char *hex, int count)
 {
   int i;
 
-  /* May use a length, or a nul-terminated string as input.  */
-  if (count == 0)
-    count = strlen ((char *) bin);
-
   for (i = 0; i < count; i++)
     {
       *hex++ = tohex ((*bin >> 4) & 0xf);
diff --git a/gdb/remote.c b/gdb/remote.c
index 881a203..fe4a9f5 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -7864,7 +7864,8 @@ extended_remote_run (char *args)
 
   if (strlen (remote_exec_file) * 2 + len >= get_remote_packet_size ())
     error (_("Remote file name too long for run packet"));
-  len += 2 * bin2hex ((gdb_byte *) remote_exec_file, rs->buf + len, 0);
+  len += 2 * bin2hex ((gdb_byte *) remote_exec_file, rs->buf + len,
+		      strlen (remote_exec_file));
 
   gdb_assert (args != NULL);
   if (*args)
@@ -7880,7 +7881,8 @@ extended_remote_run (char *args)
 	  if (strlen (argv[i]) * 2 + 1 + len >= get_remote_packet_size ())
 	    error (_("Argument list too long for run packet"));
 	  rs->buf[len++] = ';';
-	  len += 2 * bin2hex ((gdb_byte *) argv[i], rs->buf + len, 0);
+	  len += 2 * bin2hex ((gdb_byte *) argv[i], rs->buf + len,
+			      strlen (argv[i]));
 	}
       do_cleanups (back_to);
     }
@@ -8955,7 +8957,7 @@ remote_rcmd (char *command,
     error (_("\"monitor\" command ``%s'' is too long."), command);
 
   /* Encode the actual command.  */
-  bin2hex ((gdb_byte *) command, p, 0);
+  bin2hex ((gdb_byte *) command, p, strlen (command));
 
   if (putpkt (rs->buf) < 0)
     error (_("Communication problem with target."));
@@ -10572,7 +10574,7 @@ remote_download_trace_state_variable (struct trace_state_variable *tsv)
   p = rs->buf + strlen (rs->buf);
   if ((p - rs->buf) + strlen (tsv->name) * 2 >= get_remote_packet_size ())
     error (_("Trace state variable name too long for tsv definition packet"));
-  p += 2 * bin2hex ((gdb_byte *) (tsv->name), p, 0);
+  p += 2 * bin2hex ((gdb_byte *) (tsv->name), p, strlen (tsv->name));
   *p++ = '\0';
   putpkt (rs->buf);
   remote_get_noisy_reply (&target_buf, &target_buf_size);
@@ -10902,7 +10904,7 @@ remote_save_trace_data (const char *filename)
   p += strlen (p);
   if ((p - rs->buf) + strlen (filename) * 2 >= get_remote_packet_size ())
     error (_("Remote file name too long for trace save packet"));
-  p += 2 * bin2hex ((gdb_byte *) filename, p, 0);
+  p += 2 * bin2hex ((gdb_byte *) filename, p, strlen (filename));
   *p++ = '\0';
   putpkt (rs->buf);
   reply = remote_get_noisy_reply (&target_buf, &target_buf_size);
@@ -11107,21 +11109,21 @@ remote_set_trace_notes (const char *user, const char *notes,
   if (user)
     {
       buf += xsnprintf (buf, endbuf - buf, "user:");
-      nbytes = bin2hex ((gdb_byte *) user, buf, 0);
+      nbytes = bin2hex ((gdb_byte *) user, buf, strlen (user));
       buf += 2 * nbytes;
       *buf++ = ';';
     }
   if (notes)
     {
       buf += xsnprintf (buf, endbuf - buf, "notes:");
-      nbytes = bin2hex ((gdb_byte *) notes, buf, 0);
+      nbytes = bin2hex ((gdb_byte *) notes, buf, strlen (notes));
       buf += 2 * nbytes;
       *buf++ = ';';
     }
   if (stop_notes)
     {
       buf += xsnprintf (buf, endbuf - buf, "tstop:");
-      nbytes = bin2hex ((gdb_byte *) stop_notes, buf, 0);
+      nbytes = bin2hex ((gdb_byte *) stop_notes, buf, strlen (stop_notes));
       buf += 2 * nbytes;
       *buf++ = ';';
     }
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 56bfb84..a2cadac 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -3090,7 +3090,7 @@ encode_source_string (int tpnum, ULONGEST addr,
 	   srctype, 0, (int) strlen (src));
   if (strlen (buf) + strlen (src) * 2 >= buf_size)
     error (_("Source string too long for buffer"));
-  bin2hex ((gdb_byte *) src, buf + strlen (buf), 0);
+  bin2hex ((gdb_byte *) src, buf + strlen (buf), strlen (src));
   return -1;
 }
 
@@ -3209,7 +3209,7 @@ tfile_write_status (struct trace_file_writer *self,
     {
       char *buf = (char *) alloca (strlen (ts->stop_desc) * 2 + 1);
 
-      bin2hex ((gdb_byte *) ts->stop_desc, buf, 0);
+      bin2hex ((gdb_byte *) ts->stop_desc, buf, strlen (ts->stop_desc));
       fprintf (writer->fp, ":%s", buf);
     }
   fprintf (writer->fp, ":%x", ts->stopping_tracepoint);
@@ -3239,14 +3239,14 @@ tfile_write_status (struct trace_file_writer *self,
     {
       char *buf = (char *) alloca (strlen (ts->notes) * 2 + 1);
 
-      bin2hex ((gdb_byte *) ts->notes, buf, 0);
+      bin2hex ((gdb_byte *) ts->notes, buf, strlen (ts->notes));
       fprintf (writer->fp, ";notes:%s", buf);
     }
   if (ts->user_name != NULL)
     {
       char *buf = (char *) alloca (strlen (ts->user_name) * 2 + 1);
 
-      bin2hex ((gdb_byte *) ts->user_name, buf, 0);
+      bin2hex ((gdb_byte *) ts->user_name, buf, strlen (ts->user_name));
       fprintf (writer->fp, ";username:%s", buf);
     }
   fprintf (writer->fp, "\n");
@@ -3266,7 +3266,7 @@ tfile_write_uploaded_tsv (struct trace_file_writer *self,
   if (utsv->name)
     {
       buf = (char *) xmalloc (strlen (utsv->name) * 2 + 1);
-      bin2hex ((gdb_byte *) (utsv->name), buf, 0);
+      bin2hex ((gdb_byte *) (utsv->name), buf, strlen (utsv->name));
     }
 
   fprintf (writer->fp, "tsv %x:%s:%x:%s\n",
-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC 2/9] move some rsp bits into rsp-low.h
  2014-01-20 19:18 [RFC 0/9] move more code into common/ Tom Tromey
                   ` (2 preceding siblings ...)
  2014-01-20 19:18 ` [RFC 6/9] replace convert_int_to_ascii with bin2hex Tom Tromey
@ 2014-01-20 19:18 ` Tom Tromey
  2014-01-21  3:24   ` Yao Qi
  2014-02-05 20:33   ` Pedro Alves
  2014-01-20 19:23 ` [RFC 9/9] update rsp-low comments Tom Tromey
                   ` (4 subsequent siblings)
  8 siblings, 2 replies; 25+ messages in thread
From: Tom Tromey @ 2014-01-20 19:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This moves various low-level remote serial protocol bits into
common/rsp-low.[ch].

This is as close to a pure move as possible.  There are some
redundancies remaining but those will be dealt with in a subsequent
patch.

Note that the two variants of remote_escape_output disagreed on the
treatment of "*".  On the theory that quoting cannot hurt but the
absence possibly can, I chose the gdbserver variant to be the
canonical one.

2014-01-20  Tom Tromey  <tromey@redhat.com>

	* tracepoint.c: Include rsp-low.h.
	* remote.h (hex2bin, bin2hex, unpack_varlen_hex): Don't declare.
	* remote.c: Include rsp-low.h.
	(hexchars, ishex, unpack_varlen_hex, pack_nibble, pack_hex_byte)
	(fromhex, hex2bin, tohex, bin2hex, remote_escape_output)
	(remote_unescape_input): Move to common/rsp-low.c.
	* common/rsp-low.h: New file.
	* common/rsp-low.c: New file.
	* Makefile.in (SFILES): Add common/rsp-low.c.
	(HFILES_NO_SRCDIR): Add common/rsp-low.h.
	(COMMON_OBS): Add rsp-low.o.
	(rsp-low.o): New target.

2014-01-20  Tom Tromey  <tromey@redhat.com>

	* tracepoint.c: Include rsp-low.h.
	* server.c: Include rsp-low.h.
	* remote-utils.h (convert_ascii_to_int, convert_int_to_ascii)
	(unhexify, hexify, remote_escape_output, unpack_varlen_hex): Don't
	declare.
	* remote-utils.c: Include rsp-low.h.
	(fromhex, hexchars, ishex, unhexify, tohex, hexify)
	(remote_escape_output, remote_unescape_input, unpack_varlen_hex)
	(convert_int_to_ascii, convert_ascii_to_int): Move to
	common/rsp-low.c.
	* regcache.c: Include rsp-low.h.
	* ax.c: Include rsp-low.h.
	* Makefile.in (SFILES): Add common/rsp-low.c.
	(OBS): Add rsp-low.o.
	(rsp-low.o): New target.
---
 gdb/ChangeLog                |  15 +++
 gdb/Makefile.in              |  10 +-
 gdb/common/rsp-low.c         | 277 +++++++++++++++++++++++++++++++++++++++++++
 gdb/common/rsp-low.h         |  72 +++++++++++
 gdb/gdbserver/ChangeLog      |  18 +++
 gdb/gdbserver/Makefile.in    |   8 +-
 gdb/gdbserver/ax.c           |   1 +
 gdb/gdbserver/regcache.c     |   1 +
 gdb/gdbserver/remote-utils.c | 212 +--------------------------------
 gdb/gdbserver/remote-utils.h |   9 --
 gdb/gdbserver/server.c       |   1 +
 gdb/gdbserver/tracepoint.c   |   1 +
 gdb/remote.c                 | 217 +--------------------------------
 gdb/remote.h                 |   6 -
 gdb/tracepoint.c             |   1 +
 15 files changed, 402 insertions(+), 447 deletions(-)
 create mode 100644 gdb/common/rsp-low.c
 create mode 100644 gdb/common/rsp-low.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 43ff30e..e1bc975 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -780,7 +780,7 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
 	common/gdb_vecs.c common/common-utils.c common/xml-utils.c \
 	common/ptid.c common/buffer.c gdb-dlfcn.c common/agent.c \
 	common/format.c common/filestuff.c btrace.c record-btrace.c ctf.c \
-	target/waitstatus.c common/cells.c
+	target/waitstatus.c common/cells.c common/rsp-low.c
 
 LINTFILES = $(SFILES) $(YYFILES) $(CONFIG_SRCS) init.c
 
@@ -862,7 +862,7 @@ common/linux-osdata.h gdb-dlfcn.h auto-load.h probe.h stap-probe.h \
 gdb_bfd.h sparc-ravenscar-thread.h ppc-ravenscar-thread.h common/linux-btrace.h \
 ctf.h common/i386-cpuid.h common/i386-gcc-cpuid.h target/resume.h \
 target/wait.h target/waitstatus.h nat/linux-nat.h nat/linux-waitpid.h \
-common/cells.h
+common/cells.h common/rsp-low.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
@@ -960,7 +960,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	gdb_vecs.o jit.o progspace.o skip.o probe.o \
 	common-utils.o buffer.o ptid.o gdb-dlfcn.o common-agent.o \
 	format.o registry.o btrace.o record-btrace.o waitstatus.o \
-	cells.o
+	cells.o rsp-low.o
 
 TSOBS = inflow.o
 
@@ -2076,6 +2076,10 @@ cells.o: ${srcdir}/common/cells.c
 	$(COMPILE) $(srcdir)/common/cells.c
 	$(POSTCOMPILE)
 
+rsp-low.o: ${srcdir}/common/rsp-low.c
+	$(COMPILE) $(srcdir)/common/rsp-low.c
+	$(POSTCOMPILE)
+
 #
 # gdb/target/ dependencies
 #
diff --git a/gdb/common/rsp-low.c b/gdb/common/rsp-low.c
new file mode 100644
index 0000000..0e3da39
--- /dev/null
+++ b/gdb/common/rsp-low.c
@@ -0,0 +1,277 @@
+/* Low-level RSP routines for GDB, the GNU debugger.
+
+   Copyright (C) 1988-2014 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifdef GDBSERVER
+#include "server.h"
+#else
+#include "defs.h"
+#endif
+
+#include <string.h>
+
+#include "rsp-low.h"
+
+/* Convert hex digit A to a number.  */
+
+int
+fromhex (int a)
+{
+  if (a >= '0' && a <= '9')
+    return a - '0';
+  else if (a >= 'a' && a <= 'f')
+    return a - 'a' + 10;
+  else if (a >= 'A' && a <= 'F')
+    return a - 'A' + 10;
+  else
+    error (_("Reply contains invalid hex digit %d"), a);
+}
+
+int
+tohex (int nib)
+{
+  if (nib < 10)
+    return '0' + nib;
+  else
+    return 'a' + nib - 10;
+}
+
+/* Encode 64 bits in 16 chars of hex.  */
+
+static const char hexchars[] = "0123456789abcdef";
+
+static int
+ishex (int ch, int *val)
+{
+  if ((ch >= 'a') && (ch <= 'f'))
+    {
+      *val = ch - 'a' + 10;
+      return 1;
+    }
+  if ((ch >= 'A') && (ch <= 'F'))
+    {
+      *val = ch - 'A' + 10;
+      return 1;
+    }
+  if ((ch >= '0') && (ch <= '9'))
+    {
+      *val = ch - '0';
+      return 1;
+    }
+  return 0;
+}
+
+char *
+pack_nibble (char *buf, int nibble)
+{
+  *buf++ = hexchars[(nibble & 0x0f)];
+  return buf;
+}
+
+char *
+pack_hex_byte (char *pkt, int byte)
+{
+  *pkt++ = hexchars[(byte >> 4) & 0xf];
+  *pkt++ = hexchars[(byte & 0xf)];
+  return pkt;
+}
+
+char *
+unpack_varlen_hex (char *buff,	/* packet to parse */
+		   ULONGEST *result)
+{
+  int nibble;
+  ULONGEST retval = 0;
+
+  while (ishex (*buff, &nibble))
+    {
+      buff++;
+      retval = retval << 4;
+      retval |= nibble & 0x0f;
+    }
+  *result = retval;
+  return buff;
+}
+
+int
+hex2bin (const char *hex, gdb_byte *bin, int count)
+{
+  int i;
+
+  for (i = 0; i < count; i++)
+    {
+      if (hex[0] == 0 || hex[1] == 0)
+	{
+	  /* Hex string is short, or of uneven length.
+	     Return the count that has been converted so far.  */
+	  return i;
+	}
+      *bin++ = fromhex (hex[0]) * 16 + fromhex (hex[1]);
+      hex += 2;
+    }
+  return i;
+}
+
+int
+unhexify (char *bin, const char *hex, int count)
+{
+  int i;
+
+  for (i = 0; i < count; i++)
+    {
+      if (hex[0] == 0 || hex[1] == 0)
+	{
+	  /* Hex string is short, or of uneven length.
+	     Return the count that has been converted so far. */
+	  return i;
+	}
+      *bin++ = fromhex (hex[0]) * 16 + fromhex (hex[1]);
+      hex += 2;
+    }
+  return i;
+}
+
+void
+convert_ascii_to_int (const char *from, unsigned char *to, int n)
+{
+  int nib1, nib2;
+  while (n--)
+    {
+      nib1 = fromhex (*from++);
+      nib2 = fromhex (*from++);
+      *to++ = (((nib1 & 0x0f) << 4) & 0xf0) | (nib2 & 0x0f);
+    }
+}
+
+int
+bin2hex (const gdb_byte *bin, char *hex, int count)
+{
+  int i;
+
+  /* May use a length, or a nul-terminated string as input.  */
+  if (count == 0)
+    count = strlen ((char *) bin);
+
+  for (i = 0; i < count; i++)
+    {
+      *hex++ = tohex ((*bin >> 4) & 0xf);
+      *hex++ = tohex (*bin++ & 0xf);
+    }
+  *hex = 0;
+  return i;
+}
+
+int
+hexify (char *hex, const char *bin, int count)
+{
+  int i;
+
+  /* May use a length, or a nul-terminated string as input. */
+  if (count == 0)
+    count = strlen (bin);
+
+  for (i = 0; i < count; i++)
+    {
+      *hex++ = tohex ((*bin >> 4) & 0xf);
+      *hex++ = tohex (*bin++ & 0xf);
+    }
+  *hex = 0;
+  return i;
+}
+
+void
+convert_int_to_ascii (const unsigned char *from, char *to, int n)
+{
+  int nib;
+  int ch;
+  while (n--)
+    {
+      ch = *from++;
+      nib = ((ch & 0xf0) >> 4) & 0x0f;
+      *to++ = tohex (nib);
+      nib = ch & 0x0f;
+      *to++ = tohex (nib);
+    }
+  *to++ = 0;
+}
+
+int
+remote_escape_output (const gdb_byte *buffer, int len,
+		      gdb_byte *out_buf, int *out_len,
+		      int out_maxlen)
+{
+  int input_index, output_index;
+
+  output_index = 0;
+  for (input_index = 0; input_index < len; input_index++)
+    {
+      gdb_byte b = buffer[input_index];
+
+      if (b == '$' || b == '#' || b == '}' || b == '*')
+	{
+	  /* These must be escaped.  */
+	  if (output_index + 2 > out_maxlen)
+	    break;
+	  out_buf[output_index++] = '}';
+	  out_buf[output_index++] = b ^ 0x20;
+	}
+      else
+	{
+	  if (output_index + 1 > out_maxlen)
+	    break;
+	  out_buf[output_index++] = b;
+	}
+    }
+
+  *out_len = input_index;
+  return output_index;
+}
+
+int
+remote_unescape_input (const gdb_byte *buffer, int len,
+		       gdb_byte *out_buf, int out_maxlen)
+{
+  int input_index, output_index;
+  int escaped;
+
+  output_index = 0;
+  escaped = 0;
+  for (input_index = 0; input_index < len; input_index++)
+    {
+      gdb_byte b = buffer[input_index];
+
+      if (output_index + 1 > out_maxlen)
+	error ("Received too much data from the target.");
+
+      if (escaped)
+	{
+	  out_buf[output_index++] = b ^ 0x20;
+	  escaped = 0;
+	}
+      else if (b == '}')
+	escaped = 1;
+      else
+	out_buf[output_index++] = b;
+    }
+
+  if (escaped)
+    error ("Unmatched escape character in target response.");
+
+  return output_index;
+}
+
diff --git a/gdb/common/rsp-low.h b/gdb/common/rsp-low.h
new file mode 100644
index 0000000..177649a
--- /dev/null
+++ b/gdb/common/rsp-low.h
@@ -0,0 +1,72 @@
+/* Low-level RSP routines for GDB, the GNU debugger.
+
+   Copyright (C) 1988-2014 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef COMMON_RSP_LOW_H
+#define COMMON_RSP_LOW_H
+
+/* Convert hex digit A to a number, or throw an exception.  */
+
+extern int fromhex (int a);
+
+/* Convert number NIB to a hex digit.  */
+
+extern int tohex (int nib);
+
+extern char *pack_nibble (char *buf, int nibble);
+
+extern char *pack_hex_byte (char *pkt, int byte);
+
+extern char *unpack_varlen_hex (char *buff, ULONGEST *result);
+
+extern int hex2bin (const char *hex, gdb_byte *bin, int count);
+
+extern int unhexify (char *bin, const char *hex, int count);
+
+extern void convert_ascii_to_int (const char *from, unsigned char *to, int n);
+
+extern int bin2hex (const gdb_byte *bin, char *hex, int count);
+
+extern int hexify (char *hex, const char *bin, int count);
+
+extern void convert_int_to_ascii (const unsigned char *from, char *to, int n);
+
+/* Convert BUFFER, binary data at least LEN bytes long, into escaped
+   binary data in OUT_BUF.  Set *OUT_LEN to the length of the data
+   encoded in OUT_BUF, and return the number of bytes in OUT_BUF
+   (which may be more than *OUT_LEN due to escape characters).  The
+   total number of bytes in the output buffer will be at most
+   OUT_MAXLEN.  */
+
+extern int remote_escape_output (const gdb_byte *buffer, int len,
+				 gdb_byte *out_buf, int *out_len,
+				 int out_maxlen);
+
+/* Convert BUFFER, escaped data LEN bytes long, into binary data
+   in OUT_BUF.  Return the number of bytes written to OUT_BUF.
+   Raise an error if the total number of bytes exceeds OUT_MAXLEN.
+
+   This function reverses remote_escape_output.  It allows more
+   escaped characters than that function does, in particular because
+   '*' must be escaped to avoid the run-length encoding processing
+   in reading packets.  */
+
+extern int remote_unescape_input (const gdb_byte *buffer, int len,
+				  gdb_byte *out_buf, int out_maxlen);
+
+#endif /* COMMON_RSP_LOW_H */
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 60d6e2f..73c09fe 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -163,7 +163,8 @@ SFILES=	$(srcdir)/gdbreplay.c $(srcdir)/inferiors.c $(srcdir)/dll.c \
 	$(srcdir)/common/linux-osdata.c $(srcdir)/common/ptid.c \
 	$(srcdir)/common/buffer.c $(srcdir)/common/linux-btrace.c \
 	$(srcdir)/common/filestuff.c $(srcdir)/target/waitstatus.c \
-    $(srcdir)/common/mips-linux-watch.c $(srcdir)/common/cells.c
+    $(srcdir)/common/mips-linux-watch.c $(srcdir)/common/cells.c \
+	$(srcdir)/common/rsp-low.c
 
 DEPFILES = @GDBSERVER_DEPFILES@
 
@@ -176,7 +177,7 @@ OBS = agent.o ax.o inferiors.o regcache.o remote-utils.o server.o signals.o \
       target.o waitstatus.o utils.o version.o vec.o gdb_vecs.o \
       mem-break.o hostio.o event-loop.o tracepoint.o xml-utils.o \
       common-utils.o ptid.o buffer.o format.o filestuff.o dll.o notif.o \
-      tdesc.o cells.o $(XML_BUILTIN) $(DEPFILES) $(LIBOBJS)
+      tdesc.o cells.o rsp-low.o $(XML_BUILTIN) $(DEPFILES) $(LIBOBJS)
 GDBREPLAY_OBS = gdbreplay.o version.o
 GDBSERVER_LIBS = @GDBSERVER_LIBS@
 XM_CLIBS = @LIBS@
@@ -485,6 +486,9 @@ signals.o: ../common/signals.c
 cells.o: ../common/cells.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
+rsp-low.o: ../common/rsp-low.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
 linux-procfs.o: ../common/linux-procfs.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
diff --git a/gdb/gdbserver/ax.c b/gdb/gdbserver/ax.c
index d0f5b30..f62a613 100644
--- a/gdb/gdbserver/ax.c
+++ b/gdb/gdbserver/ax.c
@@ -20,6 +20,7 @@
 #include "ax.h"
 #include "format.h"
 #include "tracepoint.h"
+#include "rsp-low.h"
 
 static void ax_vdebug (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
 
diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c
index ee4e2a8..f51e498 100644
--- a/gdb/gdbserver/regcache.c
+++ b/gdb/gdbserver/regcache.c
@@ -20,6 +20,7 @@
 #include "regdef.h"
 #include "gdbthread.h"
 #include "tdesc.h"
+#include "rsp-low.h"
 
 #include <stdlib.h>
 #include <string.h>
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 75ace6e..710d398 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -22,6 +22,7 @@
 #include "gdbthread.h"
 #include "tdesc.h"
 #include "dll.h"
+#include "rsp-low.h"
 
 #include <stdio.h>
 #include <string.h>
@@ -417,66 +418,10 @@ remote_close (void)
   reset_readchar ();
 }
 
-/* Convert hex digit A to a number.  */
-
-static int
-fromhex (int a)
-{
-  if (a >= '0' && a <= '9')
-    return a - '0';
-  else if (a >= 'a' && a <= 'f')
-    return a - 'a' + 10;
-  else
-    error ("Reply contains invalid hex digit");
-  return 0;
-}
-
 #endif
 
-static const char hexchars[] = "0123456789abcdef";
-
-static int
-ishex (int ch, int *val)
-{
-  if ((ch >= 'a') && (ch <= 'f'))
-    {
-      *val = ch - 'a' + 10;
-      return 1;
-    }
-  if ((ch >= 'A') && (ch <= 'F'))
-    {
-      *val = ch - 'A' + 10;
-      return 1;
-    }
-  if ((ch >= '0') && (ch <= '9'))
-    {
-      *val = ch - '0';
-      return 1;
-    }
-  return 0;
-}
-
 #ifndef IN_PROCESS_AGENT
 
-int
-unhexify (char *bin, const char *hex, int count)
-{
-  int i;
-
-  for (i = 0; i < count; i++)
-    {
-      if (hex[0] == 0 || hex[1] == 0)
-	{
-	  /* Hex string is short, or of uneven length.
-	     Return the count that has been converted so far. */
-	  return i;
-	}
-      *bin++ = fromhex (hex[0]) * 16 + fromhex (hex[1]);
-      hex += 2;
-    }
-  return i;
-}
-
 void
 decode_address (CORE_ADDR *addrp, const char *start, int len)
 {
@@ -512,118 +457,8 @@ decode_address_to_semicolon (CORE_ADDR *addrp, const char *start)
 
 #endif
 
-/* Convert number NIB to a hex digit.  */
-
-static int
-tohex (int nib)
-{
-  if (nib < 10)
-    return '0' + nib;
-  else
-    return 'a' + nib - 10;
-}
-
 #ifndef IN_PROCESS_AGENT
 
-int
-hexify (char *hex, const char *bin, int count)
-{
-  int i;
-
-  /* May use a length, or a nul-terminated string as input. */
-  if (count == 0)
-    count = strlen (bin);
-
-  for (i = 0; i < count; i++)
-    {
-      *hex++ = tohex ((*bin >> 4) & 0xf);
-      *hex++ = tohex (*bin++ & 0xf);
-    }
-  *hex = 0;
-  return i;
-}
-
-/* Convert BUFFER, binary data at least LEN bytes long, into escaped
-   binary data in OUT_BUF.  Set *OUT_LEN to the length of the data
-   encoded in OUT_BUF, and return the number of bytes in OUT_BUF
-   (which may be more than *OUT_LEN due to escape characters).  The
-   total number of bytes in the output buffer will be at most
-   OUT_MAXLEN.  */
-
-int
-remote_escape_output (const gdb_byte *buffer, int len,
-		      gdb_byte *out_buf, int *out_len,
-		      int out_maxlen)
-{
-  int input_index, output_index;
-
-  output_index = 0;
-  for (input_index = 0; input_index < len; input_index++)
-    {
-      gdb_byte b = buffer[input_index];
-
-      if (b == '$' || b == '#' || b == '}' || b == '*')
-	{
-	  /* These must be escaped.  */
-	  if (output_index + 2 > out_maxlen)
-	    break;
-	  out_buf[output_index++] = '}';
-	  out_buf[output_index++] = b ^ 0x20;
-	}
-      else
-	{
-	  if (output_index + 1 > out_maxlen)
-	    break;
-	  out_buf[output_index++] = b;
-	}
-    }
-
-  *out_len = input_index;
-  return output_index;
-}
-
-/* Convert BUFFER, escaped data LEN bytes long, into binary data
-   in OUT_BUF.  Return the number of bytes written to OUT_BUF.
-   Raise an error if the total number of bytes exceeds OUT_MAXLEN.
-
-   This function reverses remote_escape_output.  It allows more
-   escaped characters than that function does, in particular because
-   '*' must be escaped to avoid the run-length encoding processing
-   in reading packets.  */
-
-static int
-remote_unescape_input (const gdb_byte *buffer, int len,
-		       gdb_byte *out_buf, int out_maxlen)
-{
-  int input_index, output_index;
-  int escaped;
-
-  output_index = 0;
-  escaped = 0;
-  for (input_index = 0; input_index < len; input_index++)
-    {
-      gdb_byte b = buffer[input_index];
-
-      if (output_index + 1 > out_maxlen)
-	error ("Received too much data from the target.");
-
-      if (escaped)
-	{
-	  out_buf[output_index++] = b ^ 0x20;
-	  escaped = 0;
-	}
-      else if (b == '}')
-	escaped = 1;
-      else
-	out_buf[output_index++] = b;
-    }
-
-  if (escaped)
-    error ("Unmatched escape character in target response.");
-
-  return output_index;
-}
-
 /* Look for a sequence of characters which can be run-length encoded.
    If there are any, update *CSUM and *P.  Otherwise, output the
    single character.  Return the number of characters consumed.  */
@@ -670,23 +505,6 @@ try_rle (char *buf, int remaining, unsigned char *csum, char **p)
 
 #endif
 
-char *
-unpack_varlen_hex (char *buff,	/* packet to parse */
-		   ULONGEST *result)
-{
-  int nibble;
-  ULONGEST retval = 0;
-
-  while (ishex (*buff, &nibble))
-    {
-      buff++;
-      retval = retval << 4;
-      retval |= nibble & 0x0f;
-    }
-  *result = retval;
-  return buff;
-}
-
 #ifndef IN_PROCESS_AGENT
 
 /* Write a PTID to BUF.  Returns BUF+CHARACTERS_WRITTEN.  */
@@ -1230,36 +1048,8 @@ write_enn (char *buf)
 
 #endif
 
-void
-convert_int_to_ascii (const unsigned char *from, char *to, int n)
-{
-  int nib;
-  int ch;
-  while (n--)
-    {
-      ch = *from++;
-      nib = ((ch & 0xf0) >> 4) & 0x0f;
-      *to++ = tohex (nib);
-      nib = ch & 0x0f;
-      *to++ = tohex (nib);
-    }
-  *to++ = 0;
-}
-
 #ifndef IN_PROCESS_AGENT
 
-void
-convert_ascii_to_int (const char *from, unsigned char *to, int n)
-{
-  int nib1, nib2;
-  while (n--)
-    {
-      nib1 = fromhex (*from++);
-      nib2 = fromhex (*from++);
-      *to++ = (((nib1 & 0x0f) << 4) & 0xf0) | (nib2 & 0x0f);
-    }
-}
-
 static char *
 outreg (struct regcache *regcache, int regno, char *buf)
 {
diff --git a/gdb/gdbserver/remote-utils.h b/gdb/gdbserver/remote-utils.h
index f2f1aa3..a79a35f 100644
--- a/gdb/gdbserver/remote-utils.h
+++ b/gdb/gdbserver/remote-utils.h
@@ -44,8 +44,6 @@ void initialize_async_io (void);
 void enable_async_io (void);
 void disable_async_io (void);
 void check_remote_input_interrupt_request (void);
-void convert_ascii_to_int (const char *from, unsigned char *to, int n);
-void convert_int_to_ascii (const unsigned char *from, char *to, int n);
 void new_thread_notify (int id);
 void dead_thread_notify (int id);
 void prepare_resume_reply (char *buf, ptid_t ptid,
@@ -68,13 +66,6 @@ int decode_search_memory_packet (const char *buf, int packet_len,
 				 gdb_byte *pattern,
 				 unsigned int *pattern_lenp);
 
-int unhexify (char *bin, const char *hex, int count);
-int hexify (char *hex, const char *bin, int count);
-int remote_escape_output (const gdb_byte *buffer, int len,
-			  gdb_byte *out_buf, int *out_len,
-			  int out_maxlen);
-char *unpack_varlen_hex (char *buff,  ULONGEST *result);
-
 void clear_symbol_cache (struct sym_cache **symcache_p);
 int look_up_one_symbol (const char *name, CORE_ADDR *addrp, int may_ask_gdb);
 
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 28ea048..2fe81cf 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -21,6 +21,7 @@
 #include "agent.h"
 #include "notif.h"
 #include "tdesc.h"
+#include "rsp-low.h"
 
 #include <unistd.h>
 #if HAVE_SIGNAL_H
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 13b3ff6..613220c 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -20,6 +20,7 @@
 #include "tracepoint.h"
 #include "gdbthread.h"
 #include "agent.h"
+#include "rsp-low.h"
 
 #include <ctype.h>
 #include <fcntl.h>
diff --git a/gdb/remote.c b/gdb/remote.c
index d886929..881a203 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -45,6 +45,7 @@
 #include "target-descriptions.h"
 #include "gdb_bfd.h"
 #include "filestuff.h"
+#include "rsp-low.h"
 
 #include <sys/time.h>
 
@@ -124,8 +125,6 @@ static void remote_serial_write (const char *str, int len);
 
 static void remote_kill (struct target_ops *ops);
 
-static int tohex (int nib);
-
 static int remote_can_async_p (void);
 
 static int remote_is_async_p (void);
@@ -154,8 +153,6 @@ static void init_extended_remote_ops (void);
 
 static void remote_stop (ptid_t);
 
-static int ishex (int ch, int *val);
-
 static int stubhex (int ch);
 
 static int hexnumstr (char *, ULONGEST);
@@ -176,8 +173,6 @@ static ptid_t remote_current_thread (ptid_t oldptid);
 
 static void remote_find_new_threads (void);
 
-static int fromhex (int a);
-
 static int putpkt_binary (char *buf, int cnt);
 
 static void check_binary_download (CORE_ADDR addr);
@@ -1941,14 +1936,8 @@ struct gdb_ext_thread_info
 
 #define BUF_THREAD_ID_SIZE (OPAQUETHREADBYTES * 2)
 
-char *unpack_varlen_hex (char *buff, ULONGEST *result);
-
 static char *unpack_nibble (char *buf, int *val);
 
-static char *pack_nibble (char *buf, int nibble);
-
-static char *pack_hex_byte (char *pkt, int /* unsigned char */ byte);
-
 static char *unpack_byte (char *buf, int *value);
 
 static char *pack_int (char *buf, int value);
@@ -2075,31 +2064,6 @@ read_ptid (char *buf, char **obuf)
   return ptid_build (pid, 0, tid);
 }
 
-/* Encode 64 bits in 16 chars of hex.  */
-
-static const char hexchars[] = "0123456789abcdef";
-
-static int
-ishex (int ch, int *val)
-{
-  if ((ch >= 'a') && (ch <= 'f'))
-    {
-      *val = ch - 'a' + 10;
-      return 1;
-    }
-  if ((ch >= 'A') && (ch <= 'F'))
-    {
-      *val = ch - 'A' + 10;
-      return 1;
-    }
-  if ((ch >= '0') && (ch <= '9'))
-    {
-      *val = ch - '0';
-      return 1;
-    }
-  return 0;
-}
-
 static int
 stubhex (int ch)
 {
@@ -2129,23 +2093,6 @@ stub_unpack_int (char *buff, int fieldlength)
   return retval;
 }
 
-char *
-unpack_varlen_hex (char *buff,	/* packet to parse */
-		   ULONGEST *result)
-{
-  int nibble;
-  ULONGEST retval = 0;
-
-  while (ishex (*buff, &nibble))
-    {
-      buff++;
-      retval = retval << 4;
-      retval |= nibble & 0x0f;
-    }
-  *result = retval;
-  return buff;
-}
-
 static char *
 unpack_nibble (char *buf, int *val)
 {
@@ -2154,21 +2101,6 @@ unpack_nibble (char *buf, int *val)
 }
 
 static char *
-pack_nibble (char *buf, int nibble)
-{
-  *buf++ = hexchars[(nibble & 0x0f)];
-  return buf;
-}
-
-static char *
-pack_hex_byte (char *pkt, int byte)
-{
-  *pkt++ = hexchars[(byte >> 4) & 0xf];
-  *pkt++ = hexchars[(byte & 0xf)];
-  return pkt;
-}
-
-static char *
 unpack_byte (char *buf, int *value)
 {
   *value = stub_unpack_int (buf, 2);
@@ -4669,68 +4601,6 @@ extended_remote_attach (struct target_ops *ops, char *args, int from_tty)
   extended_remote_attach_1 (ops, args, from_tty);
 }
 
-/* Convert hex digit A to a number.  */
-
-static int
-fromhex (int a)
-{
-  if (a >= '0' && a <= '9')
-    return a - '0';
-  else if (a >= 'a' && a <= 'f')
-    return a - 'a' + 10;
-  else if (a >= 'A' && a <= 'F')
-    return a - 'A' + 10;
-  else
-    error (_("Reply contains invalid hex digit %d"), a);
-}
-
-int
-hex2bin (const char *hex, gdb_byte *bin, int count)
-{
-  int i;
-
-  for (i = 0; i < count; i++)
-    {
-      if (hex[0] == 0 || hex[1] == 0)
-	{
-	  /* Hex string is short, or of uneven length.
-	     Return the count that has been converted so far.  */
-	  return i;
-	}
-      *bin++ = fromhex (hex[0]) * 16 + fromhex (hex[1]);
-      hex += 2;
-    }
-  return i;
-}
-
-/* Convert number NIB to a hex digit.  */
-
-static int
-tohex (int nib)
-{
-  if (nib < 10)
-    return '0' + nib;
-  else
-    return 'a' + nib - 10;
-}
-
-int
-bin2hex (const gdb_byte *bin, char *hex, int count)
-{
-  int i;
-
-  /* May use a length, or a nul-terminated string as input.  */
-  if (count == 0)
-    count = strlen ((char *) bin);
-
-  for (i = 0; i < count; i++)
-    {
-      *hex++ = tohex ((*bin >> 4) & 0xf);
-      *hex++ = tohex (*bin++ & 0xf);
-    }
-  *hex = 0;
-  return i;
-}
 \f
 /* Check for the availability of vCont.  This function should also check
    the response.  */
@@ -6683,91 +6553,6 @@ remote_address_masked (CORE_ADDR addr)
   return addr;
 }
 
-/* Convert BUFFER, binary data at least LEN bytes long, into escaped
-   binary data in OUT_BUF.  Set *OUT_LEN to the length of the data
-   encoded in OUT_BUF, and return the number of bytes in OUT_BUF
-   (which may be more than *OUT_LEN due to escape characters).  The
-   total number of bytes in the output buffer will be at most
-   OUT_MAXLEN.  */
-
-static int
-remote_escape_output (const gdb_byte *buffer, int len,
-		      gdb_byte *out_buf, int *out_len,
-		      int out_maxlen)
-{
-  int input_index, output_index;
-
-  output_index = 0;
-  for (input_index = 0; input_index < len; input_index++)
-    {
-      gdb_byte b = buffer[input_index];
-
-      if (b == '$' || b == '#' || b == '}')
-	{
-	  /* These must be escaped.  */
-	  if (output_index + 2 > out_maxlen)
-	    break;
-	  out_buf[output_index++] = '}';
-	  out_buf[output_index++] = b ^ 0x20;
-	}
-      else
-	{
-	  if (output_index + 1 > out_maxlen)
-	    break;
-	  out_buf[output_index++] = b;
-	}
-    }
-
-  *out_len = input_index;
-  return output_index;
-}
-
-/* Convert BUFFER, escaped data LEN bytes long, into binary data
-   in OUT_BUF.  Return the number of bytes written to OUT_BUF.
-   Raise an error if the total number of bytes exceeds OUT_MAXLEN.
-
-   This function reverses remote_escape_output.  It allows more
-   escaped characters than that function does, in particular because
-   '*' must be escaped to avoid the run-length encoding processing
-   in reading packets.  */
-
-static int
-remote_unescape_input (const gdb_byte *buffer, int len,
-		       gdb_byte *out_buf, int out_maxlen)
-{
-  int input_index, output_index;
-  int escaped;
-
-  output_index = 0;
-  escaped = 0;
-  for (input_index = 0; input_index < len; input_index++)
-    {
-      gdb_byte b = buffer[input_index];
-
-      if (output_index + 1 > out_maxlen)
-	{
-	  warning (_("Received too much data from remote target;"
-		     " ignoring overflow."));
-	  return output_index;
-	}
-
-      if (escaped)
-	{
-	  out_buf[output_index++] = b ^ 0x20;
-	  escaped = 0;
-	}
-      else if (b == '}')
-	escaped = 1;
-      else
-	out_buf[output_index++] = b;
-    }
-
-  if (escaped)
-    error (_("Unmatched escape character in target response."));
-
-  return output_index;
-}
-
 /* Determine whether the remote target supports binary downloading.
    This is accomplished by sending a no-op memory write of zero length
    to the target at the specified address. It does not suffice to send
diff --git a/gdb/remote.h b/gdb/remote.h
index 98c47b1..46b73d9 100644
--- a/gdb/remote.h
+++ b/gdb/remote.h
@@ -39,12 +39,6 @@ extern void getpkt (char **buf, long *sizeof_buf, int forever);
 
 extern int putpkt (char *buf);
 
-extern int hex2bin (const char *hex, gdb_byte *bin, int count);
-
-extern int bin2hex (const gdb_byte *bin, char *hex, int count);
-
-extern char *unpack_varlen_hex (char *buff, ULONGEST *result);
-
 void register_remote_g_packet_guess (struct gdbarch *gdbarch, int bytes,
 				     const struct target_desc *tdesc);
 void register_remote_support_xml (const char *);
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index f3b9bfc..56bfb84 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -55,6 +55,7 @@
 #include "probe.h"
 #include "ctf.h"
 #include "filestuff.h"
+#include "rsp-low.h"
 
 /* readline include files */
 #include "readline/readline.h"
-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC 0/9] move more code into common/
@ 2014-01-20 19:18 Tom Tromey
  2014-01-20 19:18 ` [RFC 1/9] share "cell" code Tom Tromey
                   ` (8 more replies)
  0 siblings, 9 replies; 25+ messages in thread
From: Tom Tromey @ 2014-01-20 19:18 UTC (permalink / raw)
  To: gdb-patches

I noticed some more code that was copied between gdb and gdbserver.
This series merges some of the easier bits: the cell-based printing
code, and various low-level RSP functions, mostly those related to
conversions to and from hex.

I split the hex conversion patch into tiny pieces to make it clearer
that each step makes sense.

Built and regtested on x86-64 Fedora 18 using the native-gdbserver
target board.

Tom

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC 8/9] replace convert_ascii_to_int with hex2bin
  2014-01-20 19:18 [RFC 0/9] move more code into common/ Tom Tromey
                   ` (6 preceding siblings ...)
  2014-01-20 19:23 ` [RFC 4/9] don't let hexify call strlen Tom Tromey
@ 2014-01-20 19:23 ` Tom Tromey
  2014-02-05 22:18   ` Pedro Alves
  2014-01-20 19:38 ` [RFC 7/9] replace unhexify " Tom Tromey
  8 siblings, 1 reply; 25+ messages in thread
From: Tom Tromey @ 2014-01-20 19:23 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

convert_ascii_to_int is identical to hex2bin.
This removes the former.

2014-01-20  Tom Tromey  <tromey@redhat.com>

	* common/rsp-low.c (convert_ascii_to_int): Remove.
	* common/rsp-low.h (convert_ascii_to_int): Don't declare.

2014-01-20  Tom Tromey  <tromey@redhat.com>

	* ax.c (gdb_parse_agent_expr): Use hex2bin, not
	convert_ascii_to_int.
	* regcache.c (registers_to_string): Likewise.
	* remote-utils.c (decode_M_packet): Likewise.
	* server.c (process_serial_event): Likewise.
---
 gdb/ChangeLog                |  5 +++++
 gdb/common/rsp-low.c         | 12 ------------
 gdb/common/rsp-low.h         |  2 --
 gdb/gdbserver/ChangeLog      |  8 ++++++++
 gdb/gdbserver/ax.c           |  2 +-
 gdb/gdbserver/regcache.c     |  2 +-
 gdb/gdbserver/remote-utils.c |  2 +-
 gdb/gdbserver/server.c       |  4 ++--
 8 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/gdb/common/rsp-low.c b/gdb/common/rsp-low.c
index 9c1a56f..d7a1280 100644
--- a/gdb/common/rsp-low.c
+++ b/gdb/common/rsp-low.c
@@ -127,18 +127,6 @@ hex2bin (const char *hex, gdb_byte *bin, int count)
   return i;
 }
 
-void
-convert_ascii_to_int (const char *from, unsigned char *to, int n)
-{
-  int nib1, nib2;
-  while (n--)
-    {
-      nib1 = fromhex (*from++);
-      nib2 = fromhex (*from++);
-      *to++ = (((nib1 & 0x0f) << 4) & 0xf0) | (nib2 & 0x0f);
-    }
-}
-
 int
 bin2hex (const gdb_byte *bin, char *hex, int count)
 {
diff --git a/gdb/common/rsp-low.h b/gdb/common/rsp-low.h
index 229b909..611dba3 100644
--- a/gdb/common/rsp-low.h
+++ b/gdb/common/rsp-low.h
@@ -36,8 +36,6 @@ extern char *unpack_varlen_hex (char *buff, ULONGEST *result);
 
 extern int hex2bin (const char *hex, gdb_byte *bin, int count);
 
-extern void convert_ascii_to_int (const char *from, unsigned char *to, int n);
-
 extern int bin2hex (const gdb_byte *bin, char *hex, int count);
 
 /* Convert BUFFER, binary data at least LEN bytes long, into escaped
diff --git a/gdb/gdbserver/ax.c b/gdb/gdbserver/ax.c
index 1eefaf3..ef659dc 100644
--- a/gdb/gdbserver/ax.c
+++ b/gdb/gdbserver/ax.c
@@ -105,7 +105,7 @@ gdb_parse_agent_expr (char **actparm)
   aexpr = xmalloc (sizeof (struct agent_expr));
   aexpr->length = xlen;
   aexpr->bytes = xmalloc (xlen);
-  convert_ascii_to_int (act, aexpr->bytes, xlen);
+  hex2bin (act, aexpr->bytes, xlen);
   *actparm = act + (xlen * 2);
   return aexpr;
 }
diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c
index 6e5aa08..bed10b48 100644
--- a/gdb/gdbserver/regcache.c
+++ b/gdb/gdbserver/regcache.c
@@ -234,7 +234,7 @@ registers_from_string (struct regcache *regcache, char *buf)
       if (len > tdesc->registers_size * 2)
 	len = tdesc->registers_size * 2;
     }
-  convert_ascii_to_int (buf, registers, len / 2);
+  hex2bin (buf, registers, len / 2);
 }
 
 struct reg *
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index a10af64..22253d4 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -1270,7 +1270,7 @@ decode_M_packet (char *from, CORE_ADDR *mem_addr_ptr, unsigned int *len_ptr,
   if (*to_p == NULL)
     *to_p = xmalloc (*len_ptr);
 
-  convert_ascii_to_int (&from[i++], *to_p, *len_ptr);
+  hex2bin (&from[i++], *to_p, *len_ptr);
 }
 
 int
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 255a048..5b4d202 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -3509,7 +3509,7 @@ process_serial_event (void)
       break;
     case 'C':
       require_running (own_buf);
-      convert_ascii_to_int (own_buf + 1, &sig, 1);
+      hex2bin (own_buf + 1, &sig, 1);
       if (gdb_signal_to_host_p (sig))
 	signal = gdb_signal_to_host (sig);
       else
@@ -3518,7 +3518,7 @@ process_serial_event (void)
       break;
     case 'S':
       require_running (own_buf);
-      convert_ascii_to_int (own_buf + 1, &sig, 1);
+      hex2bin (own_buf + 1, &sig, 1);
       if (gdb_signal_to_host_p (sig))
 	signal = gdb_signal_to_host (sig);
       else
-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC 5/9] replace hexify with bin2hex
  2014-01-20 19:18 [RFC 0/9] move more code into common/ Tom Tromey
                   ` (4 preceding siblings ...)
  2014-01-20 19:23 ` [RFC 9/9] update rsp-low comments Tom Tromey
@ 2014-01-20 19:23 ` Tom Tromey
  2014-02-05 20:38   ` Pedro Alves
  2014-01-20 19:23 ` [RFC 4/9] don't let hexify call strlen Tom Tromey
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Tom Tromey @ 2014-01-20 19:23 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This removes hexify in favor of bin2hex.
The choice of which to keep was arbitrary.

2014-01-20  Tom Tromey  <tromey@redhat.com>

	* common/rsp-low.h (hexify): Don't declare.
	* common/rsp-low.c (hexify): Remove.

2014-01-20  Tom Tromey  <tromey@redhat.com>

	* remote-utils.c (look_up_one_symbol, monitor_output): Use
	bin2hex, not hexify.
	* tracepoint.c (cmd_qtstatus): Likewise.
---
 gdb/ChangeLog                |  5 +++++
 gdb/common/rsp-low.c         | 14 --------------
 gdb/common/rsp-low.h         |  2 --
 gdb/gdbserver/ChangeLog      |  6 ++++++
 gdb/gdbserver/remote-utils.c |  5 +++--
 gdb/gdbserver/tracepoint.c   |  6 +++---
 6 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/gdb/common/rsp-low.c b/gdb/common/rsp-low.c
index feb6d0c..e288c6a 100644
--- a/gdb/common/rsp-low.c
+++ b/gdb/common/rsp-low.c
@@ -172,20 +172,6 @@ bin2hex (const gdb_byte *bin, char *hex, int count)
   return i;
 }
 
-int
-hexify (char *hex, const char *bin, int count)
-{
-  int i;
-
-  for (i = 0; i < count; i++)
-    {
-      *hex++ = tohex ((*bin >> 4) & 0xf);
-      *hex++ = tohex (*bin++ & 0xf);
-    }
-  *hex = 0;
-  return i;
-}
-
 void
 convert_int_to_ascii (const unsigned char *from, char *to, int n)
 {
diff --git a/gdb/common/rsp-low.h b/gdb/common/rsp-low.h
index 177649a..4903cd8 100644
--- a/gdb/common/rsp-low.h
+++ b/gdb/common/rsp-low.h
@@ -42,8 +42,6 @@ extern void convert_ascii_to_int (const char *from, unsigned char *to, int n);
 
 extern int bin2hex (const gdb_byte *bin, char *hex, int count);
 
-extern int hexify (char *hex, const char *bin, int count);
-
 extern void convert_int_to_ascii (const unsigned char *from, char *to, int n);
 
 /* Convert BUFFER, binary data at least LEN bytes long, into escaped
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index b2a84c3..9d0ec55 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -1399,7 +1399,8 @@ look_up_one_symbol (const char *name, CORE_ADDR *addrp, int may_ask_gdb)
 
   /* Send the request.  */
   strcpy (own_buf, "qSymbol:");
-  hexify (own_buf + strlen ("qSymbol:"), name, strlen (name));
+  bin2hex ((const gdb_byte *) name, own_buf + strlen ("qSymbol:"),
+	  strlen (name));
   if (putpkt (own_buf) < 0)
     return -1;
 
@@ -1562,7 +1563,7 @@ monitor_output (const char *msg)
   char *buf = xmalloc (len * 2 + 2);
 
   buf[0] = 'O';
-  hexify (buf + 1, msg, len);
+  bin2hex ((const gdb_byte *) msg, buf + 1, len);
 
   putpkt (buf);
   free (buf);
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 613220c..8dffe83 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -3608,17 +3608,17 @@ cmd_qtstatus (char *packet)
   str = (tracing_user_name ? tracing_user_name : "");
   slen = strlen (str);
   buf1 = (char *) alloca (slen * 2 + 1);
-  hexify (buf1, str, slen);
+  bin2hex ((gdb_byte *) str, buf1, slen);
 
   str = (tracing_notes ? tracing_notes : "");
   slen = strlen (str);
   buf2 = (char *) alloca (slen * 2 + 1);
-  hexify (buf2, str, slen);
+  bin2hex ((gdb_byte *) str, buf2, slen);
 
   str = (tracing_stop_note ? tracing_stop_note : "");
   slen = strlen (str);
   buf3 = (char *) alloca (slen * 2 + 1);
-  hexify (buf3, str, slen);
+  bin2hex ((gdb_byte *) str, buf3, slen);
 
   trace_debug ("Returning trace status as %d, stop reason %s",
 	       tracing, tracing_stop_reason);
-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC 9/9] update rsp-low comments
  2014-01-20 19:18 [RFC 0/9] move more code into common/ Tom Tromey
                   ` (3 preceding siblings ...)
  2014-01-20 19:18 ` [RFC 2/9] move some rsp bits into rsp-low.h Tom Tromey
@ 2014-01-20 19:23 ` Tom Tromey
  2014-02-05 22:22   ` Pedro Alves
  2014-01-20 19:23 ` [RFC 5/9] replace hexify with bin2hex Tom Tromey
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Tom Tromey @ 2014-01-20 19:23 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This updates all the comments in rsp-low.[ch], now that the
unification has been completed.

2014-01-20  Tom Tromey  <tromey@redhat.com>

	* common/rsp-low.c: Update comments.
	* common/rsp-low.h: Update comments.
---
 gdb/ChangeLog        |  5 +++++
 gdb/common/rsp-low.c | 18 +++++++++++++++++-
 gdb/common/rsp-low.h | 21 +++++++++++++++++++++
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/gdb/common/rsp-low.c b/gdb/common/rsp-low.c
index d7a1280..d085bd7 100644
--- a/gdb/common/rsp-low.c
+++ b/gdb/common/rsp-low.c
@@ -27,7 +27,7 @@
 
 #include "rsp-low.h"
 
-/* Convert hex digit A to a number.  */
+/* See rsp-low.h.  */
 
 int
 fromhex (int a)
@@ -42,6 +42,8 @@ fromhex (int a)
     error (_("Reply contains invalid hex digit %d"), a);
 }
 
+/* See rsp-low.h.  */
+
 int
 tohex (int nib)
 {
@@ -76,6 +78,8 @@ ishex (int ch, int *val)
   return 0;
 }
 
+/* See rsp-low.h.  */
+
 char *
 pack_nibble (char *buf, int nibble)
 {
@@ -83,6 +87,8 @@ pack_nibble (char *buf, int nibble)
   return buf;
 }
 
+/* See rsp-low.h.  */
+
 char *
 pack_hex_byte (char *pkt, int byte)
 {
@@ -91,6 +97,8 @@ pack_hex_byte (char *pkt, int byte)
   return pkt;
 }
 
+/* See rsp-low.h.  */
+
 char *
 unpack_varlen_hex (char *buff,	/* packet to parse */
 		   ULONGEST *result)
@@ -108,6 +116,8 @@ unpack_varlen_hex (char *buff,	/* packet to parse */
   return buff;
 }
 
+/* See rsp-low.h.  */
+
 int
 hex2bin (const char *hex, gdb_byte *bin, int count)
 {
@@ -127,6 +137,8 @@ hex2bin (const char *hex, gdb_byte *bin, int count)
   return i;
 }
 
+/* See rsp-low.h.  */
+
 int
 bin2hex (const gdb_byte *bin, char *hex, int count)
 {
@@ -141,6 +153,8 @@ bin2hex (const gdb_byte *bin, char *hex, int count)
   return i;
 }
 
+/* See rsp-low.h.  */
+
 int
 remote_escape_output (const gdb_byte *buffer, int len,
 		      gdb_byte *out_buf, int *out_len,
@@ -173,6 +187,8 @@ remote_escape_output (const gdb_byte *buffer, int len,
   return output_index;
 }
 
+/* See rsp-low.h.  */
+
 int
 remote_unescape_input (const gdb_byte *buffer, int len,
 		       gdb_byte *out_buf, int out_maxlen)
diff --git a/gdb/common/rsp-low.h b/gdb/common/rsp-low.h
index 611dba3..71d9dbc 100644
--- a/gdb/common/rsp-low.h
+++ b/gdb/common/rsp-low.h
@@ -28,14 +28,35 @@ extern int fromhex (int a);
 
 extern int tohex (int nib);
 
+/* Write a character representing the low order four bits of NIBBLE in
+   hex to *BUF.  Returns BUF+1.  */
+
 extern char *pack_nibble (char *buf, int nibble);
 
+/* Write the low byte of BYTE in hex to *BUF.  Returns BUF+2.  */
+
 extern char *pack_hex_byte (char *pkt, int byte);
 
+/* Read hex digits from BUFF and convert to a number, which is stored
+   in RESULT.  Reads until a non-hex digit is seen.  Returns a pointer
+   to the terminating character.  */
+
 extern char *unpack_varlen_hex (char *buff, ULONGEST *result);
 
+/* HEX is a string of characters representing hexadecimal digits.
+   Convert pairs of hex digits to bytes and store sequentially into
+   BIN.  COUNT is the maximum number of characters to convert.  This
+   will convert fewer characters if the number of hex characters
+   actually seen is odd, or if HEX terminates before COUNT characters.
+   Returns the number of characters actually converted.  */
+
 extern int hex2bin (const char *hex, gdb_byte *bin, int count);
 
+/* Convert some bytes to a hexadecimal representation.  BIN holds the
+   bytes to convert.  COUNT says how many bytes to convert.  The
+   resulting characters are stored in HEX, followed by a NUL
+   character.  Returns the number of bytes actually converted.  */
+
 extern int bin2hex (const gdb_byte *bin, char *hex, int count);
 
 /* Convert BUFFER, binary data at least LEN bytes long, into escaped
-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC 4/9] don't let hexify call strlen
  2014-01-20 19:18 [RFC 0/9] move more code into common/ Tom Tromey
                   ` (5 preceding siblings ...)
  2014-01-20 19:23 ` [RFC 5/9] replace hexify with bin2hex Tom Tromey
@ 2014-01-20 19:23 ` Tom Tromey
  2014-02-05 20:37   ` Pedro Alves
  2014-01-20 19:23 ` [RFC 8/9] replace convert_ascii_to_int with hex2bin Tom Tromey
  2014-01-20 19:38 ` [RFC 7/9] replace unhexify " Tom Tromey
  8 siblings, 1 reply; 25+ messages in thread
From: Tom Tromey @ 2014-01-20 19:23 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

hexify had the same issue as bin2hex; and the fix is the same.

2014-01-20  Tom Tromey  <tromey@redhat.com>

	* common/rsp-low.c (hexify): Never take strlen of argument.

2014-01-20  Tom Tromey  <tromey@redhat.com>

	* remote-utils.c (monitor_output): Pass explicit length to
	hexify.
---
 gdb/ChangeLog                | 4 ++++
 gdb/common/rsp-low.c         | 4 ----
 gdb/gdbserver/ChangeLog      | 5 +++++
 gdb/gdbserver/remote-utils.c | 5 +++--
 4 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/gdb/common/rsp-low.c b/gdb/common/rsp-low.c
index 9c5119b..feb6d0c 100644
--- a/gdb/common/rsp-low.c
+++ b/gdb/common/rsp-low.c
@@ -177,10 +177,6 @@ hexify (char *hex, const char *bin, int count)
 {
   int i;
 
-  /* May use a length, or a nul-terminated string as input. */
-  if (count == 0)
-    count = strlen (bin);
-
   for (i = 0; i < count; i++)
     {
       *hex++ = tohex ((*bin >> 4) & 0xf);
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 710d398..b2a84c3 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -1558,10 +1558,11 @@ relocate_instruction (CORE_ADDR *to, CORE_ADDR oldloc)
 void
 monitor_output (const char *msg)
 {
-  char *buf = xmalloc (strlen (msg) * 2 + 2);
+  int len = strlen (msg);
+  char *buf = xmalloc (len * 2 + 2);
 
   buf[0] = 'O';
-  hexify (buf + 1, msg, 0);
+  hexify (buf + 1, msg, len);
 
   putpkt (buf);
   free (buf);
-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC 7/9] replace unhexify with hex2bin
  2014-01-20 19:18 [RFC 0/9] move more code into common/ Tom Tromey
                   ` (7 preceding siblings ...)
  2014-01-20 19:23 ` [RFC 8/9] replace convert_ascii_to_int with hex2bin Tom Tromey
@ 2014-01-20 19:38 ` Tom Tromey
  2014-02-05 22:14   ` Pedro Alves
  8 siblings, 1 reply; 25+ messages in thread
From: Tom Tromey @ 2014-01-20 19:38 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

unhexify and hex2bin are identical, so this removes unhexify.  The
particular choice of which to keep was made on the basis of
parallelism with the earlier patch that removed hexify.

2014-01-20  Tom Tromey  <tromey@redhat.com>

	* common/rsp-low.h (unhexify): Don't declare.
	* common/rsp-low.c (unhexify): Remove.

2014-01-20  Tom Tromey  <tromey@redhat.com>

	* server.c (handle_query, handle_v_run): Use hex2bin, not
	unhexify.
	* tracepoint.c (cmd_qtdpsrc, cmd_qtdv, cmd_qtnotes): Likewise.
---
 gdb/ChangeLog              |  5 +++++
 gdb/common/rsp-low.c       | 19 -------------------
 gdb/common/rsp-low.h       |  2 --
 gdb/gdbserver/ChangeLog    |  6 ++++++
 gdb/gdbserver/server.c     |  5 +++--
 gdb/gdbserver/tracepoint.c | 10 +++++-----
 6 files changed, 19 insertions(+), 28 deletions(-)

diff --git a/gdb/common/rsp-low.c b/gdb/common/rsp-low.c
index ea044ee..9c1a56f 100644
--- a/gdb/common/rsp-low.c
+++ b/gdb/common/rsp-low.c
@@ -127,25 +127,6 @@ hex2bin (const char *hex, gdb_byte *bin, int count)
   return i;
 }
 
-int
-unhexify (char *bin, const char *hex, int count)
-{
-  int i;
-
-  for (i = 0; i < count; i++)
-    {
-      if (hex[0] == 0 || hex[1] == 0)
-	{
-	  /* Hex string is short, or of uneven length.
-	     Return the count that has been converted so far. */
-	  return i;
-	}
-      *bin++ = fromhex (hex[0]) * 16 + fromhex (hex[1]);
-      hex += 2;
-    }
-  return i;
-}
-
 void
 convert_ascii_to_int (const char *from, unsigned char *to, int n)
 {
diff --git a/gdb/common/rsp-low.h b/gdb/common/rsp-low.h
index 8c70dee..229b909 100644
--- a/gdb/common/rsp-low.h
+++ b/gdb/common/rsp-low.h
@@ -36,8 +36,6 @@ extern char *unpack_varlen_hex (char *buff, ULONGEST *result);
 
 extern int hex2bin (const char *hex, gdb_byte *bin, int count);
 
-extern int unhexify (char *bin, const char *hex, int count);
-
 extern void convert_ascii_to_int (const char *from, unsigned char *to, int n);
 
 extern int bin2hex (const gdb_byte *bin, char *hex, int count);
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index a53d9a2..255a048 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1931,7 +1931,8 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 	  return;
 	}
 
-      if ((len % 2) != 0 || unhexify (mon, own_buf + 6, len / 2) != len / 2)
+      if ((len % 2) != 0
+	  || hex2bin (own_buf + 6, (gdb_byte *) mon, len / 2) != len / 2)
 	{
 	  write_enn (own_buf);
 	  free (mon);
@@ -2314,7 +2315,7 @@ handle_v_run (char *own_buf)
 	{
 	  /* FIXME: Fail request if out of memory instead of dying.  */
 	  new_argv[i] = xmalloc (1 + (next_p - p) / 2);
-	  unhexify (new_argv[i], p, (next_p - p) / 2);
+	  hex2bin (p, (gdb_byte *) new_argv[i], (next_p - p) / 2);
 	  new_argv[i][(next_p - p) / 2] = '\0';
 	}
 
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 8be12ed..2585191 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -2693,7 +2693,7 @@ cmd_qtdpsrc (char *own_buf)
   packet = unpack_varlen_hex (packet, &slen);
   ++packet; /* skip a colon */
   src = xmalloc (slen + 1);
-  nbytes = unhexify (src, packet, strlen (packet) / 2);
+  nbytes = hex2bin (packet, (gdb_byte *) src, strlen (packet) / 2);
   src[nbytes] = '\0';
 
   newlast = xmalloc (sizeof (struct source_string));
@@ -2735,7 +2735,7 @@ cmd_qtdv (char *own_buf)
 
   nbytes = strlen (packet) / 2;
   varname = xmalloc (nbytes + 1);
-  nbytes = unhexify (varname, packet, nbytes);
+  nbytes = hex2bin (packet, (gdb_byte *) varname, nbytes);
   varname[nbytes] = '\0';
 
   tsv = create_trace_state_variable (num, 1);
@@ -4093,7 +4093,7 @@ cmd_qtnotes (char *own_buf)
 	  packet = strchr (packet, ';');
 	  nbytes = (packet - saved) / 2;
 	  user = xmalloc (nbytes + 1);
-	  nbytes = unhexify (user, saved, nbytes);
+	  nbytes = hex2bin (saved, (gdb_byte *) user, nbytes);
 	  user[nbytes] = '\0';
 	  ++packet; /* skip the semicolon */
 	  trace_debug ("User is '%s'", user);
@@ -4107,7 +4107,7 @@ cmd_qtnotes (char *own_buf)
 	  packet = strchr (packet, ';');
 	  nbytes = (packet - saved) / 2;
 	  notes = xmalloc (nbytes + 1);
-	  nbytes = unhexify (notes, saved, nbytes);
+	  nbytes = hex2bin (saved, (gdb_byte *) notes, nbytes);
 	  notes[nbytes] = '\0';
 	  ++packet; /* skip the semicolon */
 	  trace_debug ("Notes is '%s'", notes);
@@ -4121,7 +4121,7 @@ cmd_qtnotes (char *own_buf)
 	  packet = strchr (packet, ';');
 	  nbytes = (packet - saved) / 2;
 	  stopnote = xmalloc (nbytes + 1);
-	  nbytes = unhexify (stopnote, saved, nbytes);
+	  nbytes = hex2bin (saved, (gdb_byte *) stopnote, nbytes);
 	  stopnote[nbytes] = '\0';
 	  ++packet; /* skip the semicolon */
 	  trace_debug ("tstop note is '%s'", stopnote);
-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 2/9] move some rsp bits into rsp-low.h
  2014-01-20 19:18 ` [RFC 2/9] move some rsp bits into rsp-low.h Tom Tromey
@ 2014-01-21  3:24   ` Yao Qi
  2014-02-05 20:33   ` Pedro Alves
  1 sibling, 0 replies; 25+ messages in thread
From: Yao Qi @ 2014-01-21  3:24 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 01/21/2014 03:18 AM, Tom Tromey wrote:
> Note that the two variants of remote_escape_output disagreed on the
> treatment of "*".  On the theory that quoting cannot hurt but the
> absence possibly can, I chose the gdbserver variant to be the
> canonical one.

Right, we must escape "*", because "Responses sent by the stub must also
escape 0x2a (ascii ‘*’), so that it is not interpreted as the start of
a run-length encoded sequence (described next).", said in

https://sourceware.org/gdb/current/onlinedocs/gdb/Overview.html

> +
> +/* Convert BUFFER, escaped data LEN bytes long, into binary data
> +   in OUT_BUF.  Return the number of bytes written to OUT_BUF.
> +   Raise an error if the total number of bytes exceeds OUT_MAXLEN.
> +
> +   This function reverses remote_escape_output.  It allows more
> +   escaped characters than that function does, in particular because

I know these comments are moved mechanically, but it is out of date.
We can remove this line...

> +   '*' must be escaped to avoid the run-length encoding processing
> +   in reading packets.  */

... and move this line to remote_escape_output to explain why '*' is
escaped.

> +
> +extern int remote_unescape_input (const gdb_byte *buffer, int len,
> +				  gdb_byte *out_buf, int out_maxlen);
> +

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 1/9] share "cell" code
  2014-01-20 19:18 ` [RFC 1/9] share "cell" code Tom Tromey
@ 2014-02-05 20:05   ` Pedro Alves
  2014-02-05 20:13     ` Tom Tromey
  0 siblings, 1 reply; 25+ messages in thread
From: Pedro Alves @ 2014-02-05 20:05 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 01/20/2014 07:18 PM, Tom Tromey wrote:
> The "cell"-based printing code, like phex, was duplicated in both gdb
> and gdbserver.  This patch merges the two implementations into a new
> file in common/.
> 

Looks good to me, though I have to admit the choice of "cell"
for filenames feels a little "called for how its implemented,
rather than for what it actually serves" to me.  IOW, this is
printing stuff, and  I find it just surprising to not find it
in a file called something that at least starts with 'p', if
not 'print.h/c' even.  Failing that, perhaps pcell.h/c ?

Anyway, forward it is.  OK.

> 2014-01-20  Tom Tromey  <tromey@redhat.com>
> 
> 	* utils.c (NUMCELLS, CELLSIZE, get_cell, decimal2str, pulongest)
> 	(plongest, thirty_two, phex, phex_nz, octal2str, hex_string)
> 	(hex_string_custom, int_string, core_addr_to_string)
> 	(core_addr_to_string_nz, host_address_to_string): Move to
> 	common/cells.c.
> 	* common/cells.h: New file.
> 	* common/cells.c: New file
> 	* Makefile.in (SFILES): Add common/cells.c.
> 	(HFILES_NO_SRCDIR): Add common/cells.h.
> 	(COMMON_OBS): Add cells.o.
> 	(cells.o): New target.
> 
> 2014-01-20  Tom Tromey  <tromey@redhat.com>
> 
> 	* utils.c (NUMCELLS, CELLSIZE, get_cell, decimal2str, pulongest)
> 	(plongest, thirty_two, phex_nz): Remove.
> 	* Makefile.in (SFILES): Add common/cells.c.
> 	(OBS): Add cells.o.
> 	(cell-ipas.o): New target.
> 	(cells.o): New target.
> 	(IPA_OBJS): Add cells-ipa.o.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 1/9] share "cell" code
  2014-02-05 20:05   ` Pedro Alves
@ 2014-02-05 20:13     ` Tom Tromey
  2014-02-06  2:28       ` Yao Qi
  0 siblings, 1 reply; 25+ messages in thread
From: Tom Tromey @ 2014-02-05 20:13 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro> Looks good to me, though I have to admit the choice of "cell"
Pedro> for filenames feels a little "called for how its implemented,
Pedro> rather than for what it actually serves" to me.  IOW, this is
Pedro> printing stuff, and  I find it just surprising to not find it
Pedro> in a file called something that at least starts with 'p', if
Pedro> not 'print.h/c' even.  Failing that, perhaps pcell.h/c ?

Pedro> Anyway, forward it is.  OK.

I don't mind changing the name.

The "cell" part is an implementation detail, but an important one, since
users can rely on being able to use N such calls without worrying about
interference.

pcell seems fine to me but I am open to other suggestions.

Tom

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 2/9] move some rsp bits into rsp-low.h
  2014-01-20 19:18 ` [RFC 2/9] move some rsp bits into rsp-low.h Tom Tromey
  2014-01-21  3:24   ` Yao Qi
@ 2014-02-05 20:33   ` Pedro Alves
  2014-02-06 18:56     ` Tom Tromey
  1 sibling, 1 reply; 25+ messages in thread
From: Pedro Alves @ 2014-02-05 20:33 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 01/20/2014 07:18 PM, Tom Tromey wrote:
> This moves various low-level remote serial protocol bits into
> common/rsp-low.[ch].
> 
> This is as close to a pure move as possible.  There are some
> redundancies remaining but those will be dealt with in a subsequent
> patch.
> 
> Note that the two variants of remote_escape_output disagreed on the
> treatment of "*".  On the theory that quoting cannot hurt but the
> absence possibly can, I chose the gdbserver variant to be the
> canonical one.

Hmm.  This makes me a tiny bit nervous.  What if some random
silly stub out there is checking that the resulting byte after
unescaping is one of '#', '$' or '}'?  But maybe that's being
overzealous...

> +int
> +remote_unescape_input (const gdb_byte *buffer, int len,
> +		       gdb_byte *out_buf, int out_maxlen)
> +{
> +  int input_index, output_index;
> +  int escaped;
> +
> +  output_index = 0;
> +  escaped = 0;
> +  for (input_index = 0; input_index < len; input_index++)
> +    {
> +      gdb_byte b = buffer[input_index];
> +
> +      if (output_index + 1 > out_maxlen)
> +	error ("Received too much data from the target.");

Note this loses i18n in GDB.  Please add _().  Also, in
the GDB this particular error was actually warning, but
I think it's fine to make it an error.  I also notice that
"target" in "from the target" isn't right in the gdbserver
case, but that's a preexisting minor issue.

> +
> +      if (escaped)
> +	{
> +	  out_buf[output_index++] = b ^ 0x20;
> +	  escaped = 0;
> +	}
> +      else if (b == '}')
> +	escaped = 1;
> +      else
> +	out_buf[output_index++] = b;
> +    }
> +
> +  if (escaped)
> +    error ("Unmatched escape character in target response.");

i18n.

> +
> +  return output_index;
> +}
> +


> @@ -163,7 +163,8 @@ SFILES=	$(srcdir)/gdbreplay.c $(srcdir)/inferiors.c $(srcdir)/dll.c \
>  	$(srcdir)/common/linux-osdata.c $(srcdir)/common/ptid.c \
>  	$(srcdir)/common/buffer.c $(srcdir)/common/linux-btrace.c \
>  	$(srcdir)/common/filestuff.c $(srcdir)/target/waitstatus.c \
> -    $(srcdir)/common/mips-linux-watch.c $(srcdir)/common/cells.c
> +    $(srcdir)/common/mips-linux-watch.c $(srcdir)/common/cells.c \
> +	$(srcdir)/common/rsp-low.c

Something odd with indentation?

Otherwise, this looks fine to me.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 3/9] don't let bin2hex call strlen
  2014-01-20 19:18 ` [RFC 3/9] don't let bin2hex call strlen Tom Tromey
@ 2014-02-05 20:36   ` Pedro Alves
  0 siblings, 0 replies; 25+ messages in thread
From: Pedro Alves @ 2014-02-05 20:36 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 01/20/2014 07:18 PM, Tom Tromey wrote:
> Currently bin2hex may call strlen if the length argument is zero.
> This prevents some function unification; and also it seems cleaner to
> me not to have a special meaning for a zero length.
> 
> 2014-01-20  Tom Tromey  <tromey@redhat.com>
> 
> 	* common/rsp-low.c (bin2hex): Never take strlen of argument.
> 	* remote.c (extended_remote_run, remote_rcmd)
> 	(remote_download_trace_state_variable, remote_save_trace_data)
> 	(remote_set_trace_notes): Update.
> 	* tracepoint.c (encode_source_string, tfile_write_status)
> 	(tfile_write_uploaded_tsv): Update.

OK.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 4/9] don't let hexify call strlen
  2014-01-20 19:23 ` [RFC 4/9] don't let hexify call strlen Tom Tromey
@ 2014-02-05 20:37   ` Pedro Alves
  0 siblings, 0 replies; 25+ messages in thread
From: Pedro Alves @ 2014-02-05 20:37 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 01/20/2014 07:18 PM, Tom Tromey wrote:
> hexify had the same issue as bin2hex; and the fix is the same.
> 
> 2014-01-20  Tom Tromey  <tromey@redhat.com>
> 
> 	* common/rsp-low.c (hexify): Never take strlen of argument.
> 
> 2014-01-20  Tom Tromey  <tromey@redhat.com>
> 
> 	* remote-utils.c (monitor_output): Pass explicit length to
> 	hexify.

OK.

Thanks,
-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 5/9] replace hexify with bin2hex
  2014-01-20 19:23 ` [RFC 5/9] replace hexify with bin2hex Tom Tromey
@ 2014-02-05 20:38   ` Pedro Alves
  0 siblings, 0 replies; 25+ messages in thread
From: Pedro Alves @ 2014-02-05 20:38 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 01/20/2014 07:18 PM, Tom Tromey wrote:
> This removes hexify in favor of bin2hex.
> The choice of which to keep was arbitrary.
> 
> 2014-01-20  Tom Tromey  <tromey@redhat.com>
> 
> 	* common/rsp-low.h (hexify): Don't declare.
> 	* common/rsp-low.c (hexify): Remove.
> 
> 2014-01-20  Tom Tromey  <tromey@redhat.com>
> 
> 	* remote-utils.c (look_up_one_symbol, monitor_output): Use
> 	bin2hex, not hexify.
> 	* tracepoint.c (cmd_qtstatus): Likewise.

OK.

Thanks,
-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 6/9] replace convert_int_to_ascii with bin2hex
  2014-01-20 19:18 ` [RFC 6/9] replace convert_int_to_ascii with bin2hex Tom Tromey
@ 2014-02-05 20:39   ` Pedro Alves
  0 siblings, 0 replies; 25+ messages in thread
From: Pedro Alves @ 2014-02-05 20:39 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 01/20/2014 07:18 PM, Tom Tromey wrote:
> convert_int_to_ascii is identical to bin2hex.  This removes the
> former.  In this case I made the choice of which to keep on the basis
> that I consider the name bin2hex to be superior to
> convert_int_to_ascii.

Agreed.  This is OK.

Thanks,
-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 7/9] replace unhexify with hex2bin
  2014-01-20 19:38 ` [RFC 7/9] replace unhexify " Tom Tromey
@ 2014-02-05 22:14   ` Pedro Alves
  0 siblings, 0 replies; 25+ messages in thread
From: Pedro Alves @ 2014-02-05 22:14 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 01/20/2014 07:18 PM, Tom Tromey wrote:
> unhexify and hex2bin are identical, so this removes unhexify.  The
> particular choice of which to keep was made on the basis of
> parallelism with the earlier patch that removed hexify.
> 
> 2014-01-20  Tom Tromey  <tromey@redhat.com>
> 
> 	* common/rsp-low.h (unhexify): Don't declare.
> 	* common/rsp-low.c (unhexify): Remove.
> 
> 2014-01-20  Tom Tromey  <tromey@redhat.com>
> 
> 	* server.c (handle_query, handle_v_run): Use hex2bin, not
> 	unhexify.
> 	* tracepoint.c (cmd_qtdpsrc, cmd_qtdv, cmd_qtnotes): Likewise.

OK, thanks.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 8/9] replace convert_ascii_to_int with hex2bin
  2014-01-20 19:23 ` [RFC 8/9] replace convert_ascii_to_int with hex2bin Tom Tromey
@ 2014-02-05 22:18   ` Pedro Alves
  0 siblings, 0 replies; 25+ messages in thread
From: Pedro Alves @ 2014-02-05 22:18 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 01/20/2014 07:18 PM, Tom Tromey wrote:
> convert_ascii_to_int is identical to hex2bin.
> This removes the former.
> 
> 2014-01-20  Tom Tromey  <tromey@redhat.com>
> 
> 	* common/rsp-low.c (convert_ascii_to_int): Remove.
> 	* common/rsp-low.h (convert_ascii_to_int): Don't declare.
> 
> 2014-01-20  Tom Tromey  <tromey@redhat.com>
> 
> 	* ax.c (gdb_parse_agent_expr): Use hex2bin, not
> 	convert_ascii_to_int.
> 	* regcache.c (registers_to_string): Likewise.
> 	* remote-utils.c (decode_M_packet): Likewise.
> 	* server.c (process_serial_event): Likewise.

OK, thanks.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 9/9] update rsp-low comments
  2014-01-20 19:23 ` [RFC 9/9] update rsp-low comments Tom Tromey
@ 2014-02-05 22:22   ` Pedro Alves
  0 siblings, 0 replies; 25+ messages in thread
From: Pedro Alves @ 2014-02-05 22:22 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 01/20/2014 07:18 PM, Tom Tromey wrote:
> This updates all the comments in rsp-low.[ch], now that the
> unification has been completed.

Looks great, thank you.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 1/9] share "cell" code
  2014-02-05 20:13     ` Tom Tromey
@ 2014-02-06  2:28       ` Yao Qi
  2014-02-06 18:51         ` Tom Tromey
  0 siblings, 1 reply; 25+ messages in thread
From: Yao Qi @ 2014-02-06  2:28 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Pedro Alves, gdb-patches

On 02/06/2014 04:13 AM, Tom Tromey wrote:
> pcell seems fine to me but I am open to other suggestions.

How about print-util.c or display-util.c?  pcell is unclear unless
people have read the file.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 1/9] share "cell" code
  2014-02-06  2:28       ` Yao Qi
@ 2014-02-06 18:51         ` Tom Tromey
  0 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2014-02-06 18:51 UTC (permalink / raw)
  To: Yao Qi; +Cc: Pedro Alves, gdb-patches

Yao> How about print-util.c or display-util.c?

I went with print-utils.[ch].

Tom

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 2/9] move some rsp bits into rsp-low.h
  2014-02-05 20:33   ` Pedro Alves
@ 2014-02-06 18:56     ` Tom Tromey
  2014-02-06 19:08       ` Pedro Alves
  0 siblings, 1 reply; 25+ messages in thread
From: Tom Tromey @ 2014-02-06 18:56 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Hmm.  This makes me a tiny bit nervous.  What if some random
Pedro> silly stub out there is checking that the resulting byte after
Pedro> unescaping is one of '#', '$' or '}'?  But maybe that's being
Pedro> overzealous...

Naturally we can't rule this out, but I think that, due to the way the
RSP text is written in the manual, it is pretty unlikely:

       The binary data representation uses `7d' (ASCII `}') as an escape
    character.  Any escaped byte is transmitted as the escape character
    followed by the original character XORed with `0x20'.  For example, the
    byte `0x7d' would be transmitted as the two bytes `0x7d 0x5d'.  The
    bytes `0x23' (ASCII `#'), `0x24' (ASCII `$'), and `0x7d' (ASCII `}')
    must always be escaped.  Responses sent by the stub must also escape
    `0x2a' (ASCII `*'), so that it is not interpreted as the start of a
    run-length encoded sequence (described next).

I think it's clear from this that the rule is that any character may be
escaped; and furthermore I think anyone implementing this would tend to
treat it generically.

Pedro> Note this loses i18n in GDB.  Please add _().
[...]
Pedro> i18n.

Fixed.

>> $(srcdir)/common/buffer.c $(srcdir)/common/linux-btrace.c \
>> $(srcdir)/common/filestuff.c $(srcdir)/target/waitstatus.c \
>> -    $(srcdir)/common/mips-linux-watch.c $(srcdir)/common/cells.c
>> +    $(srcdir)/common/mips-linux-watch.c $(srcdir)/common/cells.c \
>> +	$(srcdir)/common/rsp-low.c

Pedro> Something odd with indentation?

It's pre-existing in the Makefile.
I'll fix it separately in a few.

Tom

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC 2/9] move some rsp bits into rsp-low.h
  2014-02-06 18:56     ` Tom Tromey
@ 2014-02-06 19:08       ` Pedro Alves
  0 siblings, 0 replies; 25+ messages in thread
From: Pedro Alves @ 2014-02-06 19:08 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 02/06/2014 06:56 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> Hmm.  This makes me a tiny bit nervous.  What if some random
> Pedro> silly stub out there is checking that the resulting byte after
> Pedro> unescaping is one of '#', '$' or '}'?  But maybe that's being
> Pedro> overzealous...
> 
> Naturally we can't rule this out, but I think that, due to the way the
> RSP text is written in the manual, it is pretty unlikely:
> 
>        The binary data representation uses `7d' (ASCII `}') as an escape
>     character.  Any escaped byte is transmitted as the escape character
>     followed by the original character XORed with `0x20'.  For example, the
>     byte `0x7d' would be transmitted as the two bytes `0x7d 0x5d'.  The
>     bytes `0x23' (ASCII `#'), `0x24' (ASCII `$'), and `0x7d' (ASCII `}')
>     must always be escaped.  Responses sent by the stub must also escape
>     `0x2a' (ASCII `*'), so that it is not interpreted as the start of a
>     run-length encoded sequence (described next).
> 
> I think it's clear from this that the rule is that any character may be
> escaped; and furthermore I think anyone implementing this would tend to
> treat it generically.

Alright, let's do that then.  If something breaks, hopefully people
catch it before the next release.  If not, well, people that care
should test mainline more often.  ;-)

> 
> Pedro> Note this loses i18n in GDB.  Please add _().
> [...]
> Pedro> i18n.
> 
> Fixed.
> 
>>> $(srcdir)/common/buffer.c $(srcdir)/common/linux-btrace.c \
>>> $(srcdir)/common/filestuff.c $(srcdir)/target/waitstatus.c \
>>> -    $(srcdir)/common/mips-linux-watch.c $(srcdir)/common/cells.c
>>> +    $(srcdir)/common/mips-linux-watch.c $(srcdir)/common/cells.c \
>>> +	$(srcdir)/common/rsp-low.c
> 
> Pedro> Something odd with indentation?
> 
> It's pre-existing in the Makefile.
> I'll fix it separately in a few.

Ah, thanks.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2014-02-06 19:08 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-20 19:18 [RFC 0/9] move more code into common/ Tom Tromey
2014-01-20 19:18 ` [RFC 1/9] share "cell" code Tom Tromey
2014-02-05 20:05   ` Pedro Alves
2014-02-05 20:13     ` Tom Tromey
2014-02-06  2:28       ` Yao Qi
2014-02-06 18:51         ` Tom Tromey
2014-01-20 19:18 ` [RFC 3/9] don't let bin2hex call strlen Tom Tromey
2014-02-05 20:36   ` Pedro Alves
2014-01-20 19:18 ` [RFC 6/9] replace convert_int_to_ascii with bin2hex Tom Tromey
2014-02-05 20:39   ` Pedro Alves
2014-01-20 19:18 ` [RFC 2/9] move some rsp bits into rsp-low.h Tom Tromey
2014-01-21  3:24   ` Yao Qi
2014-02-05 20:33   ` Pedro Alves
2014-02-06 18:56     ` Tom Tromey
2014-02-06 19:08       ` Pedro Alves
2014-01-20 19:23 ` [RFC 9/9] update rsp-low comments Tom Tromey
2014-02-05 22:22   ` Pedro Alves
2014-01-20 19:23 ` [RFC 5/9] replace hexify with bin2hex Tom Tromey
2014-02-05 20:38   ` Pedro Alves
2014-01-20 19:23 ` [RFC 4/9] don't let hexify call strlen Tom Tromey
2014-02-05 20:37   ` Pedro Alves
2014-01-20 19:23 ` [RFC 8/9] replace convert_ascii_to_int with hex2bin Tom Tromey
2014-02-05 22:18   ` Pedro Alves
2014-01-20 19:38 ` [RFC 7/9] replace unhexify " Tom Tromey
2014-02-05 22:14   ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).