From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32111 invoked by alias); 8 Mar 2005 11:09:58 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 31937 invoked from network); 8 Mar 2005 11:09:48 -0000 Received: from unknown (HELO mail.codesourcery.com) (65.74.133.9) by sourceware.org with SMTP; 8 Mar 2005 11:09:48 -0000 Received: (qmail 15699 invoked from network); 8 Mar 2005 11:09:46 -0000 Received: from localhost (HELO ?192.168.189.167?) (nathan@127.0.0.1) by mail.codesourcery.com with SMTP; 8 Mar 2005 11:09:46 -0000 Message-ID: <422D87F3.9040804@codesourcery.com> Date: Tue, 08 Mar 2005 11:09:00 -0000 From: Nathan Sidwell Organization: Codesourcery LLC User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.3) Gecko/20040913 MIME-Version: 1.0 To: Daniel Jacobowitz CC: gdb-patches@sources.redhat.com Subject: Re: [PATCH] remote protocol cleanups References: <4209F9CE.4030405@codesourcery.com> <20050307224517.GB28725@nevyn.them.org> In-Reply-To: <20050307224517.GB28725@nevyn.them.org> Content-Type: multipart/mixed; boundary="------------040004090104040008020608" X-SW-Source: 2005-03/txt/msg00118.txt.bz2 This is a multi-part message in MIME format. --------------040004090104040008020608 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1213 Daniel Jacobowitz wrote: > On Wed, Feb 09, 2005 at 11:53:50AM +0000, Nathan Sidwell wrote: >>2) remote_write_bytes attempts to honour get_memory_write_packet_size's >>limit, but fails in two ways when that is a very small number. a) it can >>end up with a negative byte count. b) it deducts the number of chars in >>the *maximal* length count, not the number of chars in the actual length >>count. This can result in packets sending 1 or 2 bytes fewer than they >>are limited to. > > Mostly OK, but one question. I don't get the "max (todo, 1)". If no > bytes fit, aren't we hosed? It seems like an error condition; we > shouldn't be violating the size limit. It appears the minimum size setting is somewhat arbitrary, and incorrect. /* NOTE: 16 is just chosen at random. */ #ifndef MIN_REMOTE_PACKET_SIZE #define MIN_REMOTE_PACKET_SIZE 16 #endif the minumum packet size is actually 7 - strlen ("$M,:#NN") 8 - hexnumlen (memaddr) (32 bit host) 1 - hexnumlen (len) 2 - data = 18 characters This patch fixes that size calculation. ok? nathan -- Nathan Sidwell :: http://www.codesourcery.com :: CodeSourcery LLC nathan@codesourcery.com :: http://www.planetfall.pwp.blueyonder.co.uk --------------040004090104040008020608 Content-Type: text/plain; name="remote-2.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="remote-2.patch" Content-length: 4072 2005-03-08 Nathan Sidwell * remote.c (MIN_REMOTE_PACKET_SIZE): Set to 20. (remote_fetch_registers): Allow uppercase hex when resyncing. (remote_write_bytes): Only call get_memory_write_packet_size once. Robustify packet size calculation for very small packets. Check that at least one byte will be written. Index: remote.c =================================================================== RCS file: /cvs/src/src/gdb/remote.c,v retrieving revision 1.176 diff -c -3 -p -r1.176 remote.c *** remote.c 4 Mar 2005 17:52:53 -0000 1.176 --- remote.c 8 Mar 2005 11:03:32 -0000 *************** get_memory_packet_size (struct memory_pa *** 413,421 **** #ifndef MAX_REMOTE_PACKET_SIZE #define MAX_REMOTE_PACKET_SIZE 16384 #endif ! /* NOTE: 16 is just chosen at random. */ #ifndef MIN_REMOTE_PACKET_SIZE ! #define MIN_REMOTE_PACKET_SIZE 16 #endif long what_they_get; if (config->fixed_p) --- 413,421 ---- #ifndef MAX_REMOTE_PACKET_SIZE #define MAX_REMOTE_PACKET_SIZE 16384 #endif ! /* NOTE: 20 ensures we can write at least one byte. */ #ifndef MIN_REMOTE_PACKET_SIZE ! #define MIN_REMOTE_PACKET_SIZE 20 #endif long what_they_get; if (config->fixed_p) *************** remote_fetch_registers (int regnum) *** 3332,3337 **** --- 3332,3338 ---- in the buffer is not a hex character, assume that has happened and try to fetch another packet to read. */ while ((buf[0] < '0' || buf[0] > '9') + && (buf[0] < 'A' || buf[0] > 'F') && (buf[0] < 'a' || buf[0] > 'f') && buf[0] != 'x') /* New: unavailable register value. */ { *************** remote_write_bytes (CORE_ADDR memaddr, c *** 3659,3674 **** /* Verify that the target can support a binary download. */ check_binary_download (memaddr); /* Compute the size, and then allocate space for the largest ! possible packet. Include space for an extra trailing NULL. */ ! sizeof_buf = get_memory_write_packet_size () + 1; buf = alloca (sizeof_buf); /* Compute the size of the actual payload by subtracting out the ! packet header and footer overhead: "$M,:...#nn". */ ! payload_size = (get_memory_write_packet_size () - (strlen ("$M,:#NN") ! + hexnumlen (memaddr) ! + hexnumlen (len))); /* Construct the packet header: "[MX],:". */ --- 3660,3677 ---- /* Verify that the target can support a binary download. */ check_binary_download (memaddr); + payload_size = get_memory_write_packet_size (); + /* Compute the size, and then allocate space for the largest ! possible packet. Include space for an extra trailing NUL. */ ! sizeof_buf = payload_size + 1; buf = alloca (sizeof_buf); /* Compute the size of the actual payload by subtracting out the ! packet header and footer overhead: "$M,:...#nn". ! */ ! payload_size -= strlen ("$M,:#NN"); ! payload_size -= hexnumlen (memaddr); /* Construct the packet header: "[MX],:". */ *************** remote_write_bytes (CORE_ADDR memaddr, c *** 3681,3691 **** --- 3684,3698 ---- *p++ = 'X'; /* Best guess at number of bytes that will fit. */ todo = min (len, payload_size); + payload_size -= hexnumlen (todo); + todo = min (todo, payload_size); break; case PACKET_DISABLE: *p++ = 'M'; /* Num bytes that will fit. */ todo = min (len, payload_size / 2); + payload_size -= hexnumlen (todo); + todo = min (todo, payload_size / 2); break; case PACKET_SUPPORT_UNKNOWN: internal_error (__FILE__, __LINE__, *************** remote_write_bytes (CORE_ADDR memaddr, c *** 3693,3698 **** --- 3700,3708 ---- default: internal_error (__FILE__, __LINE__, _("bad switch")); } + if (todo <= 0) + internal_error (__FILE__, __LINE__, + _("minumum packet size too small to write data")); /* Append "". */ memaddr = remote_address_masked (memaddr); --------------040004090104040008020608--