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

[Gianni Tedesco <gianni.tedesco@citrix.com>]

On Wed, 2010-07-28 at 15:57 +0100, Patrick Colp wrote:
> On 28 July 2010 10:00, Ian Jackson <Ian.Jackson@eu.citrix.com> 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.

Gianni