On Wed, 16 Oct 2019, Christian Ehrhardt wrote: > reload_profile calls get_profile_name for no particular gain, lets > remove that call. The string isn't used in that function later on > and not registered/passed anywhere. > > It can only fail if it either can't allocate or if the > virDomainDefPtr would have no uuid set (which isn't allowed). > > Thereby the only "check" it really provides is if it can allocate the > string to then free it again. > > This was initially added in [1] when the code was still in > AppArmorRestoreSecurityImageLabel (later moved) and even back then had > no further effect than described above. > > [1]: https://libvirt.org/git/?p=libvirt.git;a=blob;f=src/security/security_apparmor.c;h=16de0f26f41689e0c50481120d9f8a59ba1f4073;hb=bbaecd6a8f15345bc822ab4b79eb0955986bb2fd#l487 > > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@xxxxxxxxxxxxx> > --- > src/security/security_apparmor.c | 14 ++------------ > 1 file changed, 2 insertions(+), 12 deletions(-) > > diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c > index 75203cc43a..691833eb4b 100644 > --- a/src/security/security_apparmor.c > +++ b/src/security/security_apparmor.c > @@ -282,17 +282,12 @@ reload_profile(virSecurityManagerPtr mgr, > const char *fn, > bool append) > { > - int rc = -1; > - char *profile_name = NULL; > virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef( > def, SECURITY_APPARMOR_NAME); > > if (!secdef || !secdef->relabel) > return 0; > > - if ((profile_name = get_profile_name(def)) == NULL) > - return rc; > - > /* Update the profile only if it is loaded */ > if (profile_loaded(secdef->imagelabel) >= 0) { > if (load_profile(mgr, secdef->imagelabel, def, fn, append) < 0) { > @@ -300,15 +295,10 @@ reload_profile(virSecurityManagerPtr mgr, > _("cannot update AppArmor profile " > "\'%s\'"), > secdef->imagelabel); > - goto cleanup; > + return -1; > } > } > - > - rc = 0; > - cleanup: > - VIR_FREE(profile_name); > - > - return rc; > + return 0; > } > > static int LGTM. I don't recall why this was there initially but guessing it was obviated by a refactor at some point (perhaps before I initially submitted). -- Jamie Strandboge | http://www.canonical.com
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
