[libvirt PATCH v6 06/30] nodedev: expose internal helper for naming devices

[Erik Skultety]

On Fri, Mar 26, 2021 at 11:48:02AM -0500, Jonathon Jongsma wrote:
> Expose a helper function that can be used by udev and mdevctl to
> generate device names for node devices.
> 
> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
> Reviewed-by: Erik Skultety <eskultet at redhat.com>
> ---
>  src/node_device/node_device_driver.c | 25 +++++++++++++++++++++++++
>  src/node_device/node_device_driver.h |  6 ++++++
>  src/node_device/node_device_udev.c   | 19 +++----------------
>  3 files changed, 34 insertions(+), 16 deletions(-)
> 
> diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
> index 920fd815f2..bada5bbf00 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -978,3 +978,28 @@ nodedevRegister(void)
>      return udevNodeRegister();
>  #endif
>  }
> +
> +
> +void
> +nodeDeviceGenerateName(virNodeDeviceDef *def,
> +                       const char *subsystem,
> +                       const char *sysname,
> +                       const char *s)
> +{
> +    size_t i;
> +    g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
> +
> +    virBufferAsprintf(&buf, "%s_%s",
> +                      subsystem,
> +                      sysname);
> +
> +    if (s != NULL)
> +        virBufferAsprintf(&buf, "_%s", s);
> +
> +    def->name = virBufferContentAndReset(&buf);

Actually, there's a leak ^here than I only just noticed:

11 bytes in 1 blocks are definitely lost in loss record 222 of 3,093
==629005==    at 0x4C34F0B: malloc (vg_replace_malloc.c:307)
==629005==    by 0x59A92A5: g_malloc (in /usr/lib64/libglib-2.0.so.0.5600.4)
==629005==    by 0x59C2BE2: g_strdup (in /usr/lib64/libglib-2.0.so.0.5600.4)
==629005==    by 0x504B37C: virNodeDeviceDefParseXML (node_device_conf.c:2122)
==629005==    by 0x504BA0E: virNodeDeviceDefParseNode (node_device_conf.c:2242)
==629005==    by 0x504BADD: virNodeDeviceDefParse (node_device_conf.c:2256)
==629005==    by 0x504BB1E: virNodeDeviceDefParseString (node_device_conf.c:2270)
==629005==    by 0x262229FF: nodeDeviceDefineXML (node_device_driver.c:1290)
==629005==    by 0x520D415: virNodeDeviceDefineXML (libvirt-nodedev.c:768)
==629005==    by 0x14AEED: remoteDispatchNodeDeviceDefineXML (remote_daemon_dispatch_stubs.h:1
5032
==629005==    by 0x14AE78: remoteDispatchNodeDeviceDefineXMLHelper (remote_daemon_dispatch_stu
bs.h:15013)

What happens is that we start with something like:
    ...
    def->name = g_strdup("new_device")
    ...
and we never free it before re-writing it.

My R-b still stands.

Erik