From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 765 invoked by alias); 18 Jul 2006 13:39:56 -0000 Received: (qmail 752 invoked by uid 22791); 18 Jul 2006 13:39:55 -0000 X-Spam-Check-By: sourceware.org Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.31.1) with ESMTP; Tue, 18 Jul 2006 13:39:52 +0000 Received: from drow by nevyn.them.org with local (Exim 4.54) id 1G2pnf-0004GN-Rq; Tue, 18 Jul 2006 09:39:47 -0400 Date: Tue, 18 Jul 2006 13:39:00 -0000 From: Daniel Jacobowitz To: Mark Kettenis Cc: Vladimir Prus , gdb-patches@sources.redhat.com Subject: Re: [PATCH] zero-terminate result of target_read_alloc Message-ID: <20060718133947.GA16064@nevyn.them.org> Mail-Followup-To: Mark Kettenis , Vladimir Prus , gdb-patches@sources.redhat.com References: <200607181356.16071.vladimir@codesourcery.com> <24758.192.87.1.22.1153221922.squirrel@webmail.xs4all.nl> <20060718123436.GB14653@nevyn.them.org> <22754.192.87.1.22.1153228499.squirrel@webmail.xs4all.nl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <22754.192.87.1.22.1153228499.squirrel@webmail.xs4all.nl> User-Agent: Mutt/1.5.11+cvs20060403 X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-07/txt/msg00230.txt.bz2 On Tue, Jul 18, 2006 at 03:14:59PM +0200, Mark Kettenis wrote: > Sorry, but the whole distinction between strings and binary data is wrong. > Strings are binary data, including the terminating NUL. Volodya's patch > addresses things at the wrong level. Whatever we read using > target_read_partial() should already include the terminating NUL. Please consider that you are asserting that a design change is obvious and necessary in code that we've been working with for months. I disagree with you that it's at the wrong level; please at least listen to why. > So whatever reads the XML from the target should make sure things are > NUL-terminated, and report the total length, including the terminating > NUL. > > I guess you're thinking too much in terms of the remote protocol, where > you need to make the distinction between text and binary data because > you have to encode the latter to avoid problems with embedded NUL's. But > that's really specific to the way our remote protocol works, and therefore > should be handled completely in the remote target vector code. In fact it's exactly the opposite. Embedded NULs are not a problem with the remote protocol. It is perfectly happy to include NUL characters in the data. The characters which cause a problem are [*$#]. Text and binary data are not at all distinguished in the remote protocol interface underlying TARGET_OBJECT_*. Both are escaped in the same way, which used to be as hex, but is now a binary encoding only changing those special characters. The remote protocol doesn't care at all what's in the object. Right now it doesn't know. Later on, it will know the contents of some objects (e.g. the XML memory map) and parse them - but not inside to_xfer_partial, which is strictly a transfer mechanism. That would be a layering problem. Instead other bits of the file will call target_read_alloc to fetch objects. Here's a better way to think about the patch, I think. Consider the result of target_read_alloc to be the contents of a disk file. It might be an ELF executable or a text README file. The caller knows which sort of data it is, and will process it appropriately, but target_read_alloc is just fread in this model. It doesn't know whether the contents are text or binary. If they are text, why should they include a terminating NUL in the disk file? So with this change, the interface is friendly to consumers which wish to parse the result as binary data, and also friendly to consumers which wish to pass it to strcpy or strlen. Yes, I realize my analogy is a bit flawed in that fread doesn't do this. But how frequently have you seen fread followed by some code to add a NUL at the end of the buffer? I checked a handful of source trees I had handy, and the answer seems to be about 30% of the time; pretty substantial, and these aren't text-io-heavy applications, e.g. gcc/libcpp. -- Daniel Jacobowitz CodeSourcery