From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 121059 invoked by alias); 26 Jan 2017 17:47:55 -0000 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 Received: (qmail 121034 invoked by uid 89); 26 Jan 2017 17:47:52 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-5.1 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,URIBL_RED autolearn=ham version=3.3.2 spammy=H*i:sk:91ac79a, H*f:sk:91ac79a, H*MI:sk:91ac79a X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 26 Jan 2017 17:47:42 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9338E8048C; Thu, 26 Jan 2017 17:47:42 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v0QHlbwE002487; Thu, 26 Jan 2017 12:47:41 -0500 Subject: Re: [PATCH] Harden tests that deal with memory regions To: Luis Machado , gdb-patches@sourceware.org References: <1485206680-4402-1-git-send-email-lgustavo@codesourcery.com> <830534b5-d820-d6dd-3d80-3644fcf50f5e@redhat.com> <91ac79a4-986c-38f5-8ee1-16c764b8c563@codesourcery.com> From: Pedro Alves Message-ID: Date: Thu, 26 Jan 2017 17:47:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <91ac79a4-986c-38f5-8ee1-16c764b8c563@codesourcery.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-01/txt/msg00596.txt.bz2 On 01/26/2017 05:37 PM, Luis Machado wrote: > On 01/26/2017 07:17 AM, Pedro Alves wrote: >> On 01/23/2017 09:24 PM, Luis Machado wrote: >> >>> 2017-01-23 Luis Machado >>> >>> * lib/gdb-memory.exp: New file. >> >> Do we need "gdb-" in the file name? >> >> What other procedures to you envision being placed here? Should >> this have "regions" in the file name, like "memory-regions.exp"? >> The file's intro comment talks about memory regions. >> > > I guess we don't really need the gdb prefix. I originally envisioned > this particular file storing all proc's dealing with memory checks and > manipulation (though i ended up describing it in a different way). So can you drop it? > I wanted to avoid having to add more helper functions to lib/gdb.exp. > But maybe it wouldn't be a big problem? My instinct is to modularize it. Sure, I'm not arguing against modularizing. Only against calling it "memory", but describing it as "memory ranges". I'm arguing for picking one and being consistent throughout. > Either way is fine with me though, lib/gdb.exp or lib/memory.exp. > >>> * lib/gdb.exp: Load gdb-memory.exp >> >> Missing period. >> > > Thanks. > >>> --- a/gdb/testsuite/gdb.base/breakpoint-in-ro-region.exp >>> +++ b/gdb/testsuite/gdb.base/breakpoint-in-ro-region.exp >>> @@ -137,6 +137,9 @@ if ![get_function_bounds "main" main_lo main_hi] { >>> return -1 >>> } >>> >>> +# Delete all memory regions. >>> +delete_memory_regions >>> + >> >> The comment as-is practically just reads the function name >> in English. The important detail missing here >> is "target-supplied". So: >> >> # Delete all target-supplied memory regions. >> delete_memory_regions >> >> Likewise in the other spot. >> > > On second thought, i've pulled these comments from the test files. The > updated proc documentation should be enough. What do you think? I don't think so. The important detail is that you call it _here_, right after starting the target to get rid of any target-supplied memory region. While the procedure could be called at any other point, to delete user-defined regions, even. >>> + >>> +# This file holds functions and data dealing with memory regions >>> manipulation. >>> + >>> +# Deletes all the memory regions GDB currently knows about. >>> + >>> +proc delete_memory_regions {} { > > I've added the target-supplied bit to this as well. That doesn't sound right. The procedure deletes all memory regions, either target-supplied, or user defined. Thanks, Pedro Alves