From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10827 invoked by alias); 28 May 2005 22:18:03 -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 10815 invoked by uid 22791); 28 May 2005 22:18:01 -0000 Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.30-dev) with ESMTP; Sat, 28 May 2005 22:18:01 +0000 Received: from drow by nevyn.them.org with local (Exim 4.50) id 1Dc9cv-00068H-NI; Sat, 28 May 2005 18:17:53 -0400 Date: Sat, 28 May 2005 22:55:00 -0000 From: Daniel Jacobowitz To: Michael Snyder Cc: GDB Patches Subject: Re: [rfa] Restore "trust-readonly-section" Message-ID: <20050528221753.GB22435@nevyn.them.org> Mail-Followup-To: Michael Snyder , GDB Patches References: <00c201c55745$a651dd30$aaa56b80@msnyder8600> <00e201c55748$533cea10$aaa56b80@msnyder8600> <20050515171254.GC11855@nevyn.them.org> <42927D75.4050009@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <42927D75.4050009@redhat.com> User-Agent: Mutt/1.5.8i X-SW-Source: 2005-05/txt/msg00591.txt.bz2 On Mon, May 23, 2005 at 06:03:49PM -0700, Michael Snyder wrote: > I suppose you're right -- but I'm bewildered by the number of > possible entry points and paths thru this code. In theory > I think I could get away with just covering three points: > target_read_memory, target_read_partial, and target_read_memory_partial, > were it not for the fact that do_xfer_memory is also an entry point. Yes, there really are too many. I hope that some day soon, we can reduce the total number. > do_xfer_memory seems to only be called from dcache.c -- > but it's extern, so nothing prevents others from calling it. > > And dcache_xfer_memory is extern too, so one can get in thru there > (although again, no one currently does AFAICT). > > >The code might be simpler if you push the trust_readonly check inside > >target_read_trusted. Also, could you name that something involving > >memory? > > OK -- how about the attached, which includes four entry points? This looks much nicer to me. Thaks for revising! One style concern; I'd be happier if you avoided assignments in if statement. But I'm not much bothered about it either way. > + /* Honor "trust-readonly-sections" if set. */ > + if ((ret = target_read_memory_trusted (memaddr, myaddr, len)) > 0) > + return (ret != len); > + This could be: if (target_read_memory_trusted (memaddr, myaddr, len) == len) return 0; If it fails, might as well fall through. -- Daniel Jacobowitz CodeSourcery, LLC