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

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