Re: [Xen-devel] [PATCH 2 of 3] xenpaging: Fix-up xenpaging tool code

[Patrick Colp <pjcolp@xxxxxxxxx>]

On 28 July 2010 11:01, Gianni Tedesco <gianni.tedesco@xxxxxxxxxx> wrote:
> On Wed, 2010-07-28 at 15:57 +0100, Patrick Colp wrote:
>> On 28 July 2010 10:00, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx> wrote:
>> > Patrick Colp writes ("[Xen-devel] [PATCH 2 of 3] xenpaging: Fix-up 
>> > xenpaging tool code"):
>> >>   err:
>> >> -    if ( paging->bitmap )
>> >> -        free(paging->bitmap);
>> >> -    if ( paging->platform_info )
>> >> -        free(paging->platform_info);
>> >>      if ( paging )
>> >> +    {
>> >> +        if ( paging->bitmap )
>> >> +            free(paging->bitmap);
>> >
>> > While you're doing this, why not replace
>> >
>> >>-+        if ( paging->bitmap )
>> >>-+            free(paging->bitmap);
>> > with
>> >
>> >>++        free(paging->bitmap);
>> >
>> > since free(0) is legal and a no-op ?
>>
>> Could do, but free(0) isn't exactly a no-op. free() does a check to
>> see if the pointer passed was 0. So it doesn't really make much
>> difference if I do the check or let it do the check. I can easily
>> change the code to just do free(paging->bitmap) though, if that's the
>> preferred way to do it.
>
> It's just simpler and takes less screen space.
>
>> Actually, I would argue my way is better since
>> in the case of a NULL pointer, the free function isn't called at all,
>> which saves a bunch of cycles.
>
> At the expense of expanding the binary image with a few more
> instructions. Besides don't "optimize" what isn't a bottleneck.

All good points. I'll fix up the patches and resubmit them.


Patrick

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