[PATCH 00/55] Hyper-V: code cleanup & prep for future changes
Matt Coleman
mcoleman at datto.com
Fri Jan 22 17:26:16 UTC 2021
> On Jan 22, 2021, at 12:12 PM, Matt Coleman <mcoleman at datto.com> wrote:
>
>> On Jan 22, 2021, at 12:07 PM, Matt Coleman <mcoleman at datto.com> wrote:
>>
>>> On Jan 22, 2021, at 11:30 AM, Laine Stump <laine at redhat.com> wrote:
>>>
>>> On 1/22/21 11:05 AM, Laine Stump wrote:
>>>> 1) There are several cleanup functions in external libraries that in the past were only called after checking that the pointer was != NULL. g_autoptr cleanups need to handle being called with NULL as a NOP, and I'm concerned that these functions may not behave properly in that case. Can you either verify that it's safe to call them with NULL, or provide a wrapper function that checks for NULL and use that as the cleanup?
>>>
>>> I asked about item (2) on IRC just now, and danpb produced a short example program that proves it is okay to use values from auto-freed objects as the return value of a function. So there is only question (1) left. Let me know and I'll either push or wait for modified patches accordingly.
>>
>> For WsXmlDocH, GLib's documentation says that
>> G_DEFINE_AUTOPTR_CLEANUP_FUNC() is NULL-safe:
>>> The function will not be called if the variable to be cleaned up contains NULL.
>>
>> https://developer.gnome.org/glib/stable/glib-Miscellaneous-Macros.html#G-DEFINE-AUTOPTR-CLEANUP-FUNC:CAPS
>>
>> For client_opt_t and filter_t, the third parameter to
>> G_DEFINE_AUTO_CLEANUP_FREE_FUNC() was used to prevent the types' free
>> functions from being called if they're NULL.
>
> Oh, woops. Caffeine hasn't kicked in. I got that all backwards.
>
> There is an issue with client_opt_t, which doesn't have a NULL check:
> https://github.com/Openwsman/openwsman/blob/bcf670dba4d81ec1669cfd1dd7e2b57f0957e534/src/lib/wsman-client.c#L361
>
> filter_t does have a NULL check:
> https://github.com/Openwsman/openwsman/blob/bcf670dba4d81ec1669cfd1dd7e2b57f0957e534/src/lib/wsman-filter.c#L250
>
> --
> Matt
Evidently, I'm not the sharpest knife in the drawer today: this is
NULL-safe as it is.
For WsXmlDocH, G_DEFINE_AUTO_CLEANUP_FREE_FUNC's third parameter ensures that the cleanup function won't be called if the variable is NULL:
https://developer.gnome.org/glib/stable/glib-Miscellaneous-Macros.html#G-DEFINE-AUTO-CLEANUP-FREE-FUNC:CAPS
On top of that, its cleanup function also includes a NULL check:
https://github.com/Openwsman/openwsman/blob/bcf670dba4d81ec1669cfd1dd7e2b57f0957e534/src/lib/wsman-xml.c#L663
For client_opt_t and filter_t, G_DEFINE_AUTOPTR_CLEANUP_FUNC expands to code that contains a NULL check:
https://developer.gnome.org/glib/stable/glib-Miscellaneous-Macros.html#G-DEFINE-AUTOPTR-CLEANUP-FUNC:CAPS
--
Matt
More information about the libvir-list
mailing list