Re: [PATCH] cp caching

[Jim Meyering]

Ondrej Oprala wrote:
> I'd like to propose a patch based on this bugzilla
> https://bugzilla.redhat.com/show_bug.cgi?id=806055 .
> Since it doesn't add any new functionality or fix incorrect behaviour,
> I assume there's no need for another test or a NEWS entry.
>
> From e95a3ce63cfee94bcf8099da3a093bda64521873 Mon Sep 17 00:00:00 2001
> From: Ondrej Oprala <address@hidden>
> Date: Mon, 17 Dec 2012 16:35:29 +0100
> Subject: [PATCH] cp: cache security context
>
> *src/copy.c (copy_internal): Add a condition to only
> call setfscreatecon if the security context has changed.

Good idea.

> ---
>  src/copy.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/src/copy.c b/src/copy.c
> index 60322b7..5675f8c 100644
> --- a/src/copy.c
> +++ b/src/copy.c
> @@ -2201,11 +2201,19 @@ copy_internal (char const *src_name, char const 
> *dst_name,
>      {
>        bool all_errors = !x->data_copy_required || 
> x->require_preserve_context;
>        bool some_errors = !all_errors && !x->reduce_diagnostics;

However, both of these declarations belong inside the if {...} body.
Also, please change the name and meaning of s/con_eq/context_change/
When you see both/all uses being negated, it's generally good to
adjust the definition and name to avoid the need for that.

> +      bool con_eq = 0;
>        security_context_t con;
> +      static security_context_t old_con;
>
>        if (0 <= lgetfilecon (src_name, &con))
>          {
> -          if (setfscreatecon (con) < 0)
> +          con_eq = old_con && STREQ (con, old_con);
> +          if (!con_eq)
> +            {
> +              free (old_con);
> +              old_con = xstrdup (con);
> +            }
> +          if (!con_eq && setfscreatecon (con) < 0)
>              {
>                if (all_errors || (some_errors && !errno_unsupported (errno)))
>                  error (0, errno,