On Sat, 14 Dec 2019 at 15:56, Philippe Mathieu-Daudé <philmd@xxxxxxxxxx> wrote: > > Hi, > > In this series we use coccinelle to replace: > - memory_region_add_subregion_overlap(..., priority=0) > + memory_region_add_subregion(...) > > Rationale is the code is easier to read, and reviewers don't > have to worry about overlapping because it isn't used. So our implementation of these two functions makes them have the same behaviour, but the documentation comments in memory.h describe them as different: a subregion added with memory_region_add_subregion() is not supposed to overlap any other subregion unless that other subregion was explicitly marked as overlapping. My intention with the API design here was that using the _overlap() version is a statement of intent -- this region is *expected* to be overlapping with some other regions, which hopefully have a priority set so they go at the right order wrt this one. Use of the non-overlap function says "I don't expect this to overlap". (It doesn't actually assert that it doesn't overlap because we have some legacy uses, notably in the x86 PC machines, which do overlap without using the right function, which we've never tried to tidy up.) We used to have some #if-ed out code in memory.c which was able to detect incorrect overlap, but it got removed in commit b613597819587. I thought then and still do that rather than removing code and API distinctions that allow us to tell if the board code has done something wrong (unintentional overlap, especially unintentional overlap at the same priority value) it would be better to fix the board bugs so we could enable the warnings/asserts... thanks -- PMM
