bug#29225: Tramp backup-by-copying in a Converting ACL Invalid argument error on Windows 7

[Michael Albinus <michael.albinus@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 1166 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

Hi Eli,

> I think it should return nil when acl_errno_valid returns false, and
> otherwise signal an error.  It currently calls acl_errno_valid in one
> of the two places where error could happen, but not in the other.  And
> the doc string should be amended to say that.  WDYT?

Seems to work, yes. acl_from_text returns EINVAL for wrong ACLs, so the
error would be suppressed, because acl_errno_valid returns false this case.

I've tested successfully the appended patch on GNU/Linux.

(set-file-acl "/tmp/123" "dummy")

returns nil with the applied patch, and (file-error "Converting ACL"
"Invalid argument" "/tmp/123") w/o the patch.

I cannot test on MS Windows.

I don't know whether we must extend the docstring. Summarizing probable
errors is not the default in Emacs docstrings. And the docstring does
not promise, that no error at all will happen.

If you don't object, I'll commit the patch to emacs-26. Plus minor
changes in Tramp. Maybe I'll find also the time next days, to add the
missing tests for file-acl and set-file-acl in tramp-tests.el. See the
TODO list at the end of that file.

Best regards, Michael.


[-- Attachment #2: Type: text/plain, Size: 384 bytes --]

diff --git a/src/fileio.c b/src/fileio.c
index cc1399e1bd..f538f12f9b 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -3047,7 +3047,7 @@ support.  */)
   if (STRINGP (acl_string))
     {
       acl = acl_from_text (SSDATA (acl_string));
-      if (acl == NULL)
+      if (acl == NULL && acl_errno_valid (errno))
 	{
 	  report_file_error ("Converting ACL", absname);
 	  return Qnil;