Re: [PATCH 6/9] security: use g_autofree and remove unnecessary label

[Jonathon Jongsma <jjongsma@xxxxxxxxxx>]

On 1/5/23 6:26 AM, Jiang Jiacheng wrote:

...


> @@ -476,33 +459,29 @@ AppArmorGetSecurityProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
>   {
>       int rc = -1;

'rc' variable doesn't seem to be used anymore.

>       int status;
> -    char *profile_name = NULL;
> +    g_autofree char *profile_name = NULL;
>   
>       if ((profile_name = get_profile_name(def)) == NULL)
> -        return rc;
> +        return -1;
>   
>       status = profile_status(profile_name, 1);
>       if (status < -1) {
>           virReportError(VIR_ERR_INTERNAL_ERROR,
>                          "%s", _("error getting profile status"));
> -        goto cleanup;
> +        return -1;
>       } else if (status == -1) {
>           sec->label[0] = '\0';
>       } else {
>           if (virStrcpy(sec->label, profile_name, VIR_SECURITY_LABEL_BUFLEN) < 0) {
>               virReportError(VIR_ERR_INTERNAL_ERROR,
>                              "%s", _("error copying profile name"));
> -            goto cleanup;
> +            return -1;
>           }
>       }
>   
>       sec->enforcing = status == 1;
> -    rc = 0;
> -
> - cleanup:
> -    VIR_FREE(profile_name);
>   
> -    return rc;
> +    return 0;
>   }
>   
>   /* Called on VM shutdown and destroy. See AppArmorGenSecurityLabel (above) for
> @@ -555,7 +534,7 @@ AppArmorSetSecurityProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
>                                   virDomainDef *def)
>   {
>       int rc = -1;

same here. Remove?

> -    char *profile_name = NULL;
> +    g_autofree char *profile_name = NULL;
>       virSecurityLabelDef *secdef =
>           virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
>   
> @@ -563,7 +542,7 @@ AppArmorSetSecurityProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
>           return 0;
>   
>       if ((profile_name = get_profile_name(def)) == NULL)
> -        return rc;
> +        return -1;
>   
>       if (STRNEQ(SECURITY_APPARMOR_NAME, secdef->model)) {
>           virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -572,21 +551,17 @@ AppArmorSetSecurityProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
>                            "hypervisor driver is \'%s\'."),
>                          secdef->model, SECURITY_APPARMOR_NAME);
>           if (use_apparmor() > 0)
> -            goto cleanup;
> +            return -1;
>       }
>   
>       VIR_DEBUG("Changing AppArmor profile to %s", profile_name);
>       if (aa_change_profile(profile_name) < 0) {
>           virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                          _("error calling aa_change_profile()"));
> -        goto cleanup;
> +        return -1;
>       }
> -    rc = 0;
> -
> - cleanup:
> -    VIR_FREE(profile_name);
>   
> -    return rc;
> +    return 0;
>   }
>   
>   /* Called directly by API user prior to virCommandRun().
> @@ -600,8 +575,8 @@ AppArmorSetSecurityChildProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
>                                        virCommand *cmd)
>   {
>       int rc = -1;

...and again

> -    char *profile_name = NULL;
> -    char *cmd_str = NULL;
> +    g_autofree char *profile_name = NULL;
> +    g_autofree char *cmd_str = NULL;
>       virSecurityLabelDef *secdef =
>           virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
>   
> @@ -615,21 +590,17 @@ AppArmorSetSecurityChildProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
>                            "hypervisor driver is \'%s\'."),
>                          secdef->model, SECURITY_APPARMOR_NAME);
>           if (use_apparmor() > 0)
> -            goto cleanup;
> +            return -1;
>       }
>   
>       if ((profile_name = get_profile_name(def)) == NULL)
> -        goto cleanup;
> +        return -1;
>   
>       cmd_str = virCommandToString(cmd, false);
>       VIR_DEBUG("Changing AppArmor profile to %s on %s", profile_name, cmd_str);
>       virCommandSetAppArmorProfile(cmd, profile_name);
> -    rc = 0;
>   
> - cleanup:
> -    VIR_FREE(profile_name);
> -    VIR_FREE(cmd_str);
> -    return rc;
> +    return 0;
>   }
>   
>   static int