Subject: Re: bug#21699: 24.5; Bug in backup-buffer-copy and/or set-file-extended-attributes etc [set-file-extended-attributes]

From: Eli Zaretskii <[email-masked]>
Date: Fri, 23 Oct 2015 11:25:45 +0300

> Date: Thu, 22 Oct 2015 01:43:35 -0400
> From: Eli Barzilay <[email-masked]>
> Cc: [email-masked]
> 
> On Mon, Oct 19, 2015 at 6:14 AM, Eli Zaretskii <[email-masked]> wrote:
> >
> > Thanks.  I'll let others to express opinions on this alternative vs
> > the one I committed.  The difference is what happens when all the
> > attribute values are "null" values: your version returns t in that
> > case, and I'm not sure that's correct, see below.
> 
> Ah, you're talking about this code from `backup-buffer-copy':
> 
>     (unless (and extended-attributes
>                  (with-demoted-errors
>                    (set-file-extended-attributes to-name extended-attributes)))
>       ...)
> 
> In that case, I think that my slightly earlier fix which made
> `file-extended-attributes' drop "null" values is actually fine: it means
> that in the above snipped `extended-attributes' will be nil, and the
> chmod code will run.  There is another use of a similar pattern (look
> for an "If set-file-extended-attributes fails" comment which appears in
> both places) where this second one should also have the same `and'.
> 
> (The current state is messy anyway, since with your current fix, the
> `and' in the above is not needed, and anyway, `extended-attributes' is
> never nil.)
> 
> FWIW, there is no real loss of information for doing that:
> `extended-attributes' currently adds acl and selinux entries always,
> with my fix (of dropping the no-info values) you can tell when there was
> no information for acl/selinux just by the fact that there is no such
> element in the `extended-attributes' result.

Paul (or anyone else), any second opinions about this?

Thanks.