Re: [PATCH v8 3/8] xen/arm: return int *_dcache_va_range

[Ian Campbell <Ian.Campbell@citrix.com>]

On Mon, 2014-10-20 at 16:23 +0100, Jan Beulich wrote:
> >>> On 20.10.14 at 16:57, <Ian.Campbell@citrix.com> wrote:
> >> --- a/xen/arch/arm/mm.c
> >> +++ b/xen/arch/arm/mm.c
> >> @@ -388,7 +388,7 @@ void flush_page_to_ram(unsigned long mfn)
> >>  {
> >>      void *v = map_domain_page(mfn);
> >>  
> >> -    clean_and_invalidate_dcache_va_range(v, PAGE_SIZE);
> >> +    ASSERT(clean_and_invalidate_dcache_va_range(v, PAGE_SIZE) == 0);
> > 
> > Just this one assert? What about all the other callers of
> > *_dcache_va_range?
> 
> I'm glad I looked at this reply (I skipped the original one because of
> being ARM-only): Uses of ASSERT() like above won't work in non-
> debug builds. You have to latch the result into a variable and assert
> on it. The fundamental rule is: Expressions passed to ASSERT() must
> never have side effects. That's different from BUG_ON(), but even
> there Keir advocates not using expressions with side effects, aiui
> both to avoid the above mistake (i.e. getting used to doing it the
> right way) and to leave open the option of adding a build option to
> eliminate the BUG()s and BUG_ON()s too.

Yes, I don't know how I missed this!


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