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

Osier Yang jyang at redhat.com
Mon Sep 9 15:59:56 UTC 2013


On 09/09/13 19:24, Ján Tomko wrote:
> 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.
>

Not really, a dangling pointer always has potential risks. It's never 
known whether
if it will be used later, especially when the memory it pointed to is 
allocated for some
other purpose. Causing crash is actually the better result than 
unexpected ones.

Osier





More information about the libvir-list mailing list