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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
