On Thu, May 22, 2025 at 03:19:18PM +0200, Peter Krempa via Devel wrote:
On Thu, May 22, 2025 at 14:25:13 +0200, Michal Privoznik via Devel
wrote:
> From: Michal Privoznik <mprivozn(a)redhat.com>
>
> While we do not want the nss plugin to link with anything but
> necessary libs (libc and libjson-c) it can benefit from automatic
> memory freeing. Instead of inventing macros with new name for
> them, lets stick with g_autofree and g_steal_pointer() which we
> are used to from the rest of the code. Borrow and simplify
> definitions for these macros then.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> build-aux/syntax-check.mk | 2 +-
> tools/nss/libvirt_nss.h | 24 ++++++++++++++++++++++++
> 2 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
> index 62e1604e94..1303a0ce7e 100644
> --- a/build-aux/syntax-check.mk
> +++ b/build-aux/syntax-check.mk
> @@ -1421,7 +1421,7 @@ exclude_file_name_regexp--sc_prohibit_canonicalize_file_name =
\
> ^(build-aux/syntax-check\.mk|tests/virfilemock\.c)$$
>
> exclude_file_name_regexp--sc_prohibit_raw_allocation = \
> -
^(docs/advanced-tests\.rst|src/util/viralloc\.[ch]|examples/.*|tests/(securityselinuxhelper|(vircgroup|nss)mock|commandhelper)\.c|tools/wireshark/src/packet-libvirt\.c|tools/nss/libvirt_nss(_leases|_macs)?\.c)$$
> +
^(docs/advanced-tests\.rst|src/util/viralloc\.[ch]|examples/.*|tests/(securityselinuxhelper|(vircgroup|nss)mock|commandhelper)\.c|tools/wireshark/src/packet-libvirt\.c|tools/nss/libvirt_nss(_leases|_macs)?\.[ch])$$
>
> exclude_file_name_regexp--sc_prohibit_readlink = \
> ^src/(util/virutil|lxc/lxc_container)\.c$$
> diff --git a/tools/nss/libvirt_nss.h b/tools/nss/libvirt_nss.h
> index 5f356618f3..a43731de45 100644
> --- a/tools/nss/libvirt_nss.h
> +++ b/tools/nss/libvirt_nss.h
> @@ -29,6 +29,7 @@
> #include <sys/socket.h>
> #include <netinet/in.h>
> #include <netdb.h>
> +#include <stdlib.h>
>
>
> #if 0
> @@ -62,6 +63,29 @@ do { \
> # define NSS_NAME(s) _nss_libvirt_guest_##s##_r
> #endif
>
> +#if !defined(g_autofree)
> +static inline void
> +generic_free(void *p)
> +{
> + free(*((void **)p));
> +}
> +# define g_autofree __attribute__((cleanup(generic_free)))
> +#endif
> +
> +#if !defined(g_steal_pointer)
> +static inline void *
> +g_steal_pointer(void *p)
> +{
> + void **pp = (void **)p;
> + void *ptr = *pp;
> +
> + *pp = NULL;
> + return ptr;
> +}
> +# define g_steal_pointer(x) (__typeof__(*(x))) g_steal_pointer(x)
As noted in the next patch it's a bad idea to name this same as
glib macros. This code is supposed to be kept glib free and this makes
it seem as if glib was used here.
I take the opposite view - having common terminology for the same concept
across the codebase is more important as it reduces the learning ramp for
contributors.
We avoid accidental use of other parts of glib by simply not having the
header file present, so it is pretty quickly identified that you can't
use arbitrary glib code if you were to try it.
With regards,
Daniel
--
|:
https://berrange.com -o-
https://www.flickr.com/photos/dberrange :|
|:
https://libvirt.org -o-
https://fstop138.berrange.com :|
|:
https://entangle-photo.org -o-
https://www.instagram.com/dberrange :|