On Fri, May 15, 2026 at 03:06:26PM -0400, Laine Stump wrote:
On 5/14/26 9:56 AM, Martin Kletzander via Devel wrote:From: Martin Kletzander <[email protected]>This reverts commit 443c79dd7f7d4051fc0084baaa6c56a55d2aace4.Sigh. *raises hand in shame* :-/ The commit log message *sounds* reasonable; too bad it wasn't telling the full truth - the pointer *is* a local in all cases, but it doesn't point to memory that was allocated in the scope of the function - it just duplicates a pointer within a higher level struct (in these cases the privateData) that does stick around after the function ends.
I did not check whether the issue started at this point, it might have been that the double-free happened way layer after some other changes, it's just that this particular commit was ideal to revert.
(Some of the changes didn't need to be reverted (those g_free()ing memory pointed to by a member of an object that is itself being g_free()ed (or now VIR_FREE()ed) in the immediately following code), but on the other hand after seeing this disastrous effect of a "simple cleanup", maybe *all* g_free()s should just be changed to VIR_FREE() anyway - it's currently evenly balanced between g_free() and VIR_FREE() (just under 2000 occurrences each (with about 450 uses of g_clear_pointer combined with a *Free() function or g_free() itself).
They are not needed, but it reads better when they are not mixed, and this is not a code that would run each 10ms or something. Not to mention the latency of vmware soap api. But if you feel like changing it so that anything under *p is just g_free()'d and *p VIR_FREE()'d, I'd be completely fine with that.
signature.asc
Description: PGP signature
