Re: [Xen-devel] [PATCH 1 of 5] xentrace: fix t_info_pages calculation for the default case

["Jan Beulich" <JBeulich@xxxxxxxxxx>]

>>> On 23.03.11 at 12:20, Olaf Hering <olaf@xxxxxxxxx> wrote:
> On Wed, Mar 23, Keir Fraser wrote:
> 
>> On 23/03/2011 10:12, "Jan Beulich" <JBeulich@xxxxxxxxxx> wrote:
>> 
>> >>      t_info_pages /= PAGE_SIZE;
>> >> -    if ( t_info_pages % PAGE_SIZE )
>> >> +    if ( t_info_pages % PAGE_SIZE || t_info_pages == 0 )
>> > 
>> > While certainly not having a significant effect, to the unsuspecting
>> > reader this looks like a bug - is it really meant to be a remainder
>> > operation on the *result* of a division (rather than on the original
>> > dividend)? Couldn't you just (ab)use PFN_UP() here?
>> 
>> By which you mean to replace the division and subsequent if statement with
>> t_info_pages = PFN_UP(t_info_pages).
> 
> I did not know about PFN_UP() until now, using it would work as well.

Note that one caveat with using it here is that you'd first need to
move its definition from include/asm-x86/page.h to somewhere
under include/xen/ - perhaps having a pfn.h like Linux does would
make sense.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel