Re: [PATCH RESEND 07/20] security_selinux.c: modernize set/restore hostdev subsys label functions

[Laine Stump <laine@xxxxxxxxxx>]

Saying "modernize" in the title line is kind of odd - when I read that I 
thought it must be changing something related to selinux, not just 
switching to g_autoptr. How about you just say that in the first line 
(use g_auto* in security_selinux.c, or something like that).

Aside from that:

Reviewed-by: Laine Stump <laine@xxxxxxxxxx>

On 1/18/21 2:53 PM, Daniel Henrique Barboza wrote:
> Use g_auto* cleanup to avoid free() calls.
>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>
> ---
>   src/security/security_selinux.c | 54 ++++++++++-----------------------
>   1 file changed, 16 insertions(+), 38 deletions(-)
>
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index 76edaed027..99adf08a15 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -2085,7 +2085,7 @@ virSecuritySELinuxSetHostdevSubsysLabel(virSecurityManagerPtr mgr,
>   
>       switch ((virDomainHostdevSubsysType)dev->source.subsys.type) {
>       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
> -        virUSBDevicePtr usb;
> +        g_autoptr(virUSBDevice) usb = NULL;
>   
>           if (dev->missing)
>               return 0;
> @@ -2097,39 +2097,34 @@ virSecuritySELinuxSetHostdevSubsysLabel(virSecurityManagerPtr mgr,
>               return -1;
>   
>           ret = virUSBDeviceFileIterate(usb, virSecuritySELinuxSetUSBLabel, &data);
> -        virUSBDeviceFree(usb);
>           break;
>       }
>   
>       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
> -        virPCIDevicePtr pci =
> -            virPCIDeviceNew(&pcisrc->addr);
> +        g_autoptr(virPCIDevice) pci = virPCIDeviceNew(&pcisrc->addr);
>   
>           if (!pci)
>               return -1;
>   
>           if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) {
> -            char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci);
> +            g_autofree char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci);
>   
> -            if (!vfioGroupDev) {
> -                virPCIDeviceFree(pci);
> +            if (!vfioGroupDev)
>                   return -1;
> -            }
> +
>               ret = virSecuritySELinuxSetHostdevLabelHelper(vfioGroupDev,
>                                                             false,
>                                                             &data);
> -            VIR_FREE(vfioGroupDev);
>           } else {
>               ret = virPCIDeviceFileIterate(pci, virSecuritySELinuxSetPCILabel, &data);
>           }
> -        virPCIDeviceFree(pci);
>           break;
>       }
>   
>       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: {
>           virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host;
>   
> -        virSCSIDevicePtr scsi =
> +        g_autoptr(virSCSIDevice) scsi =
>               virSCSIDeviceNew(NULL,
>                                scsihostsrc->adapter, scsihostsrc->bus,
>                                scsihostsrc->target, scsihostsrc->unit,
> @@ -2141,13 +2136,11 @@ virSecuritySELinuxSetHostdevSubsysLabel(virSecurityManagerPtr mgr,
>           ret = virSCSIDeviceFileIterate(scsi,
>                                          virSecuritySELinuxSetSCSILabel,
>                                          &data);
> -        virSCSIDeviceFree(scsi);
> -
>           break;
>       }
>   
>       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST: {
> -        virSCSIVHostDevicePtr host = virSCSIVHostDeviceNew(hostsrc->wwpn);
> +        g_autoptr(virSCSIVHostDevice) host = virSCSIVHostDeviceNew(hostsrc->wwpn);
>   
>           if (!host)
>               return -1;
> @@ -2155,19 +2148,16 @@ virSecuritySELinuxSetHostdevSubsysLabel(virSecurityManagerPtr mgr,
>           ret = virSCSIVHostDeviceFileIterate(host,
>                                               virSecuritySELinuxSetHostLabel,
>                                               &data);
> -        virSCSIVHostDeviceFree(host);
>           break;
>       }
>   
>       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
> -        char *vfiodev = NULL;
> +        g_autofree char *vfiodev = NULL;
>   
>           if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr)))
>               return ret;
>   
>           ret = virSecuritySELinuxSetHostdevLabelHelper(vfiodev, true, &data);
> -
> -        VIR_FREE(vfiodev);
>           break;
>       }
>   
> @@ -2323,7 +2313,7 @@ virSecuritySELinuxRestoreHostdevSubsysLabel(virSecurityManagerPtr mgr,
>   
>       switch ((virDomainHostdevSubsysType)dev->source.subsys.type) {
>       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
> -        virUSBDevicePtr usb;
> +        g_autoptr(virUSBDevice) usb = NULL;
>   
>           if (dev->missing)
>               return 0;
> @@ -2335,37 +2325,31 @@ virSecuritySELinuxRestoreHostdevSubsysLabel(virSecurityManagerPtr mgr,
>               return -1;
>   
>           ret = virUSBDeviceFileIterate(usb, virSecuritySELinuxRestoreUSBLabel, mgr);
> -        virUSBDeviceFree(usb);
> -
>           break;
>       }
>   
>       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
> -        virPCIDevicePtr pci =
> -            virPCIDeviceNew(&pcisrc->addr);
> +        g_autoptr(virPCIDevice) pci = virPCIDeviceNew(&pcisrc->addr);
>   
>           if (!pci)
>               return -1;
>   
>           if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) {
> -            char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci);
> +            g_autofree char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci);
>   
> -            if (!vfioGroupDev) {
> -                virPCIDeviceFree(pci);
> +            if (!vfioGroupDev)
>                   return -1;
> -            }
> +
>               ret = virSecuritySELinuxRestoreFileLabel(mgr, vfioGroupDev, false);
> -            VIR_FREE(vfioGroupDev);
>           } else {
>               ret = virPCIDeviceFileIterate(pci, virSecuritySELinuxRestorePCILabel, mgr);
>           }
> -        virPCIDeviceFree(pci);
>           break;
>       }
>   
>       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: {
>           virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host;
> -        virSCSIDevicePtr scsi =
> +        g_autoptr(virSCSIDevice) scsi =
>               virSCSIDeviceNew(NULL,
>                                scsihostsrc->adapter, scsihostsrc->bus,
>                                scsihostsrc->target, scsihostsrc->unit,
> @@ -2375,13 +2359,11 @@ virSecuritySELinuxRestoreHostdevSubsysLabel(virSecurityManagerPtr mgr,
>               return -1;
>   
>           ret = virSCSIDeviceFileIterate(scsi, virSecuritySELinuxRestoreSCSILabel, mgr);
> -        virSCSIDeviceFree(scsi);
> -
>           break;
>       }
>   
>       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST: {
> -        virSCSIVHostDevicePtr host = virSCSIVHostDeviceNew(hostsrc->wwpn);
> +        g_autoptr(virSCSIVHostDevice) host = virSCSIVHostDeviceNew(hostsrc->wwpn);
>   
>           if (!host)
>               return -1;
> @@ -2389,20 +2371,16 @@ virSecuritySELinuxRestoreHostdevSubsysLabel(virSecurityManagerPtr mgr,
>           ret = virSCSIVHostDeviceFileIterate(host,
>                                               virSecuritySELinuxRestoreHostLabel,
>                                               mgr);
> -        virSCSIVHostDeviceFree(host);
> -
>           break;
>       }
>   
>       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
> -        char *vfiodev = NULL;
> +        g_autofree char *vfiodev = NULL;
>   
>           if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr)))
>               return -1;
>   
>           ret = virSecuritySELinuxRestoreFileLabel(mgr, vfiodev, true);
> -
> -        VIR_FREE(vfiodev);
>           break;
>       }
>