Re: [Xen-devel] [PATCH] slightly consolidate code in free_domheap_pages()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.06.14 at 13:27, <Ian.Campbell@xxxxxxxxxxxxx> wrote:
> On Tue, 2014-06-24 at 11:25 +0100, Jan Beulich wrote:
>> >>> On 24.06.14 at 12:04, <Ian.Campbell@xxxxxxxxxxxxx> wrote:
>> > If so, can d at this point ever be anything other than dom_cow or NULL?
>> > I don't think so. Given that I think ASSERT(!(d == dom_cow && order !=
>> > 0)) would more clearly capture the intent of the test (with the spelling
>> > out of the conditions being more important than the de morganing of the
>> > expression).
>> 
>> Indeed, d can only be NULL or dom_cow here (being in the else part
>> of the if() you quoted at the top). So an alternative might indeed be
>> ASSERT(d != dom_cow || !order), but that seems less desirable to
>> me as it opens up ways to pass the ASSERT() with d != NULL should
>> the if() condition ever get modified. I.e. I'd prefer the assertion to be
>> as restrictive as possible, getting relaxed only when in fact necessary.
> 
> Since the original if involves d == dom_cow but nothing to do with order
> it seemed that the check was somehow specific to dom_cow's relationship
> to higher order allocations.
> 
> I suppose the question is what relationship would a non-NULL d have to
> the order of the allocation. i.e. if the if were changed to also
> consider dom_foo why would we expect now that dom_foo had any order
> requirements?

We won't know, but by having it the way it is now in the patch we're
on the safe side (nothing unintended will slip through), whereas if we
change to comparing against dom_cow a not sufficiently careful future
change may introduce an issue.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel