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

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.