Re: [PATCH] Harden tests that deal with memory regions

[Luis Machado <lgustavo@codesourcery.com>]

On 01/26/2017 11:47 AM, Pedro Alves wrote:
> 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  <lgustavo@codesourcery.com>
>>>>
>>>>     * 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.
>

Great. I've picked lib/memory.exp as the name in v3.

>> 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.
>

I've reverted this in v3 and adjusted the comments to be more sensible 
depending on the context of the call.

>>>> +
>>>> +# 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
>

Fixed.

Thanks,
Luis