Re: [gold][patch] Control caching of memory mapped regions

[Ian Lance Taylor <iant at google dot com>]

Cary Coutant <ccoutant@google.com> writes:

>>> - Â Â Âif (p->second->is_locked())
>>> + Â Â Âif (p->second->is_locked() || !p->second->is_data_owned())
>>> Â Â Â should_delete = false;
>>
>> I'm having a little trouble understanding the test of is_data_owned().
>> is_data_owned() will return true if the data_ownership_ field is not
>> DATA_NOT_OWNED. ÂThe only time that field will be DATA_NOT_OWNED is
>> for the whole file view created for an in-memory file, which is only
>> done for testing. ÂSo in normal usage is_data_owned() will always
>> return true. ÂGiven the tests above, that seems to suggest that
>> should_delete is pretty much always going to be false. ÂThat is
>> obviously not what your results show, so what am I missing?
>
> I think you're missing the "!" -- is_data_owned() will always return
> true in normal execution, so !is_data_owned() will be false, and the
> condition remains as before: simply if (p->second->is_locked()). I had
> to add that just to keep it from deleting the whole file view in the
> unit tests. Yeah, the double negative is confusing; I welcome
> suggestions for a better name -- maybe something like
> is_permanent_view() or is_testing_view()?

Argh, you're right, the double negative got me.  Sorry about that.

The patch is OK.  It's also OK if you change the name.

Thanks.

Ian