From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30968 invoked by alias); 23 Mar 2010 18:16:21 -0000 Received: (qmail 30960 invoked by uid 22791); 23 Mar 2010 18:16:20 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from smtp-outbound-2.vmware.com (HELO smtp-outbound-2.vmware.com) (65.115.85.73) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 23 Mar 2010 18:16:16 +0000 Received: from jupiter.vmware.com (mailhost5.vmware.com [10.16.68.131]) by smtp-outbound-2.vmware.com (Postfix) with ESMTP id C9FB0B079; Tue, 23 Mar 2010 11:16:13 -0700 (PDT) Received: from [10.20.124.100] (promd-2s-dhcp100.eng.vmware.com [10.20.124.100]) by jupiter.vmware.com (Postfix) with ESMTP id C1BA2DC236; Tue, 23 Mar 2010 11:16:13 -0700 (PDT) Message-ID: <4BA9056D.8050206@vmware.com> Date: Tue, 23 Mar 2010 18:16:00 -0000 From: Michael Snyder User-Agent: Thunderbird 2.0.0.22 (X11/20090609) MIME-Version: 1.0 To: Pedro Alves CC: "gdb-patches@sourceware.org" Subject: Re: [RFA] gdbserver support for qCRC: (compare-sections) References: <4BA40867.4090703@vmware.com> <4BA7AFFD.6050308@vmware.com> <201003231626.13757.pedro@codesourcery.com> In-Reply-To: <201003231626.13757.pedro@codesourcery.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-03/txt/msg00793.txt.bz2 Pedro Alves wrote: > On Monday 22 March 2010 17:59:25, Michael Snyder wrote: >> 2010-03-19 Michael Snyder >> >> * 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