This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] gdbserver support for qCRC: (compare-sections)


Pedro Alves wrote:
On Monday 22 March 2010 17:59:25, Michael Snyder wrote:
2010-03-19 Michael Snyder <msnyder@vmware.com>

        * server.c (crc32): New function.
        (handle_query): Add handling for 'qCRC:' request.

Index: server.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.c,v
retrieving revision 1.108
diff -u -p -r1.108 server.c
--- server.c 20 Jan 2010 22:55:38 -0000 1.108
+++ server.c 22 Mar 2010 17:57:32 -0000
@@ -788,6 +788,47 @@ handle_threads_qxfer (const char *annex,
}
+/* Table used by the crc32 function to calcuate the checksum. */
+
+static unsigned long crc32_table[256] =
+{0, 0};
+

I know you've copied this from remote.c, but, can't we make this an `unsigned int' table? longs are 64-bit on most 64-bit archs, so this uses up double more memory than needed.

Good idea. Doesn't seem to break anything. Will do.





+/* Compute 32 bit CRC from inferior memory.
+
+   On success, return 32 bit CRC.
+   On failure, return (unsigned long long) -1.  */
+
+static unsigned long long
+crc32 (CORE_ADDR base, int len, unsigned int crc)

Can't say I'm thrilled by assuming sizeof (long long) > sizeof (long), but I guess it works on all targets gdbserver cares for presently.

And with your suggested change, the assumption now is that sizeof(long long) > sizeof(int), which is even more safe.



+  if (strncmp ("qCRC:", own_buf, 5) == 0)
+    {
+      /* CRC check (compare-segment).  */
+      char *comma;
+      CORE_ADDR base = strtoul (own_buf + 5, &comma, 16);
+      int len;
+      unsigned long long crc;
+

A `require_running' call is missing here.

Sorry, what's that? I'm not familiar with it.




+      if (*comma++ != ',')
+       {
+         write_enn (own_buf);
+         return;
+       }
+      len = strtoul (comma, NULL, 16);
+      crc = crc32 (base, len, 0xffffffff);
+      /* Check for memory failure.  */
+      if (crc == (unsigned long long) -1)
+       {
+         write_enn (own_buf);
+         return;
+       }
+      sprintf (own_buf, "C%lx", (unsigned long) crc);
+      return;
+    }
+
   /* Otherwise we didn't know what packet it was.  Say we didn't
      understand it.  */
   own_buf[0] = 0;

Otherwise, looks fine to me.

Big thanks for the review!


Michael




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]