[libvirt] [PATCH 2/4] apparmor: drop useless call to get_profile_name

Jamie Strandboge jamie at canonical.com
Tue Nov 19 20:48:26 UTC 2019


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 at canonical.com>
> ---
>  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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20191119/990a70f9/attachment-0001.sig>


More information about the libvir-list mailing list