From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7142 invoked by alias); 9 Aug 2012 08:11:36 -0000 Received: (qmail 7133 invoked by uid 22791); 9 Aug 2012 08:11:34 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,MSGID_MULTIPLE_AT,RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from service87.mimecast.com (HELO service87.mimecast.com) (91.220.42.44) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 09 Aug 2012 08:11:21 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.21]) by service87.mimecast.com; Thu, 09 Aug 2012 09:11:19 +0100 Received: from shawin053 ([10.164.6.42]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.0); Thu, 9 Aug 2012 09:12:54 +0100 From: "Terry Guo" To: "'Jonathan Larmour'" Cc: References: <000401cd7540$59679610$0c36c230$@guo@arm.com> <5023559B.1070709@eCosCentric.com> In-Reply-To: <5023559B.1070709@eCosCentric.com> Subject: RE: [Patch] Enable GDB remote protocol to support zlib-compressed xml Date: Thu, 09 Aug 2012 08:11:00 -0000 Message-ID: <000101cd7606$c6ed1540$54c73fc0$@guo@arm.com> MIME-Version: 1.0 X-MC-Unique: 112080909111905301 Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable 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: 2012-08/txt/msg00280.txt.bz2 Hi Jonathan, Your comments are great helpful. Thanks very much. =20 >=20 > In the documentation, may I suggest some slightly clearer wording: >=20 > -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- > @item compressedXfer > The remote stub understands the > @samp{qXfer:object:zread:annex:offset,length} > packet, which is similar to @samp{qXfer:object:read:annex:offset:length} > (@pxref{qXfer read}) but with the object contents in the response > packet > compressed as a zlib deflated stream, prepended with the uncompressed > length > of the whole object. The length in the @samp{qXfer:zread} request > refers to > the maximum allowed packet size. The uncompressed length is > represented as a > 64-bit value in little-endian byte order (that is, with the first byte > being > the least significant byte of the value, and the eighth byte being the > most > significant byte of the value). >=20 > It is not compulsory for a remote stub to allow compressed responses > for all > responses: if this feature is set, the host GDB will always try > @samp{zread} > first and fall back to plain @samp{read} if a null response is received > from > the remote stub. > -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- > and later: > -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- > @var{zread} works in a similar way to @var{read} except that the > transferred > objects must be compressed as a zlib deflated stream, prepended by the > uncompressed length of the object. This can allow remote stubs to > reduce > their memory footprint by returning pre-generated zlib-compressed data, > as > well as reducing communications overhead. @var{zread} can only be used > when > the host gdb is built with Zlib support. > -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- >=20 Much better than mine. I will use them. > Although thinking about the uncompressed length, every other value in > GDB's > remote protocol which is endian specific always uses big endian, not > little > endian. So I recommend changing that (and if so, also my suggested > replacement > documentation wording above). >=20 OK. Will go with big endian assumption. > For the code, I don't think you can call from target.c into remote.c > for > remote_packet_is_comprs_xfer(). remote is only one target vector. I > think the > GDB maintainers would consider this a layering violation, but hopefully > one of > them will be able to confirm for definite. I think the only sensible > alternative is to perform the decompression within remote_read_qxfer. >=20 I also think it is a layering violation and want to avoid it. But I couldn't find a way.=20 Suppose we have a very large target.xml file which needs multiple transfers, the stub may has two options to go: a) compress it as a whole and then transfer it piece by piece=20 or=20 b) split and compress each pieces and then transfer them one by one. If we go with option a), the remote_read_qxfer can't help because each received packets are just part of a big zlib-compressed object. Only function target_read_alloc_1 knows the end of the transfer of this big file. If we go with option b), the remote_read_qxfer can help because it knows each received packets are zlib-compressed. > The code currently assumes that once we know whether compressed data is > or is > not supported for a particular packet, that setting is used for all > objects > used with that packet. Is that a good assumption? Just because one > object is > compressed, must we insist that all objects are compressed? Or vice > versa. > This is somewhat theoretical given that initially we are only concerned > with > features.xml, but if this is to be a general extension, this > requirement for > the stub should be clear (and documented). >=20 Maybe my code isn't so clear. But my intentions are: a). The compressedXfer is a general stub feature and not specific to any kind of gdb packets. Suppose the object for PACKET_qXfer_osdata will be transferred in zlib-compressed format while the object for PACKET_qXfer_libraries is still in plain format, once I know the status of PACKET_qXfer_osdata, I won't assume same status for PACKET_qXfer_libraries, I will still try zread first to detect its actual status. b). If the object for PACKET_qXfer_libraries is very big and needs multiple transfer, I will assume the remaining parts will be transferred in compressed format once I know the first part is transferred in compressed format. > What about remote_write_qxfer()? >=20 I think the remote_wirte_qxfer means host gdb transfers zlib-compressed data to gdb stub. This will need stub to spare more memory for zlib functions. I can start to do it once the solution for read is accepted and there are stubs that support decompress function. Otherwise I can't get a stub to debug my code. > In gdb_zlib_error() I don't think zlib is likely to be using > stdin/stdout, so > I doubt the handling of Z_ERRNO is correct. And in any case, there > should have > been an 'else' clause to handle errors which aren't ferrors for > stdin/stdout > anyway. >=20 > You also need to handle Z_BUF_ERROR. >=20 OK. I will update the way to handle Z_ERRNO. BR, Terry