Re: [PATCH] virBitmapFree: Change the function to a macro

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/09/2013 05:57 AM, Liuji (Jeremy) wrote:
> The virBitmapFree is:
>         void virBitmapFree(virBitmapPtr bitmap)
>         {
>             if (bitmap) {
>                 VIR_FREE(bitmap->map);
>                 VIR_FREE(bitmap);
>             }
>         }
> Bitmap is a parameter (formal parameter). The address which is pointed by Bitmap is the same as argument(actual parameter) pointed.
> In " VIR_FREE(bitmap);", it frees the same address. But only assigns the NULL to the Bitmap, not to the argument(actual parameter).
> After calling virBitmapFree, if there is not assigning NULL to the argument(actual parameter), and using this argument(actual parameter) to call
> the virBitmapFree again, it will free an already freed address. It will generate a crash.
> 
> There are 3 solutions:
> 1> After call virBitmapFree function, add a assignment which is assign NULL to the argument(actual parameter) of virBitmapFree.
> 2> Change the virBitmapFree to :
>         void virBitmapFree(virBitmapPtr *bitmap)
>         {
>             if (*bitmap) {
>                 VIR_FREE((*bitmap)->map);
>                 VIR_FREE(*bitmap);
>             }
>         }
> And change all virBitmapFree calling from 
>        virBitmapFree(abc)
> to
>        virBitmapFree(&abc)
> 3> change the virBitmapFree to a macro, like my first mail.
> 
> 
> Because the 1 and 2 solutions will modify the code in many code segment. So, I prefer the 3 solution.

Most of the code doesn't use the bitmap after it's freed, it seems it's just
the one case of numatune nodemask that needs to be fixed. I vote for option
(1). I think it's better to be consistent with what other *Free functions do.

Jan

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]