From 63e2dd28eac912dcf1b55814106609138eba090b Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@k1024.org>
Date: Wed, 11 Dec 2019 21:04:42 +0100
Subject: [PATCH] Fix yet another bug in ACL re-inistialisation

---
 NEWS               |  3 +++
 acl.c              | 36 +++++++++++++++++-------------------
 tests/test_acls.py | 12 ++++++++++++
 3 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/NEWS b/NEWS
index a6a0df1..84ec3bb 100644
--- a/NEWS
+++ b/NEWS
@@ -25,6 +25,9 @@ Important API changes/bug fixes:
   non-initialised object attribute access. Note ACL re-initialisation is
   tricky and (still) leads to undefined behaviour of existing Entry
   objects pointing to it.
+- Fix another bug in ACL re-initialisation where failures would result
+  in invalid objects; now failed re-initialisation does not touch the
+  original object.
 - Restore `__setstate__`/`__getstate__` support on Linux; this was
   inadvertently removed due a typo(!) when adding support for it in
   FreeBSD. Pickle should work again for ACL instances, although not sure
diff --git a/acl.c b/acl.c
index 99f3e9a..c280967 100644
--- a/acl.c
+++ b/acl.c
@@ -146,6 +146,7 @@ static int ACL_init(PyObject* obj, PyObject* args, PyObject *keywds) {
       "y#"
 #endif
       ;
+    acl_t new = NULL;
     int mode = -1;
     PyObject *file = NULL;
     PyObject *filedef = NULL;
@@ -174,52 +175,49 @@ static int ACL_init(PyObject* obj, PyObject* args, PyObject *keywds) {
                                     ))
         return -1;
 
-    /* Free the old acl_t without checking for error, we don't
-     * care right now */
-    if(self->acl != NULL)
-        acl_free(self->acl);
-
-    self->acl = NULL;
-
     if(file != NULL) {
         fprintf(stderr, "foobar!\n");
         char *path = PyBytes_AS_STRING(file);
-        self->acl = acl_get_file(path, ACL_TYPE_ACCESS);
+        new = acl_get_file(path, ACL_TYPE_ACCESS);
         Py_DECREF(file);
     } else if(text != NULL)
-        self->acl = acl_from_text(text);
+        new = acl_from_text(text);
     else if(fd != NULL) {
         int fdval;
         if ((fdval = PyObject_AsFileDescriptor(fd)) != -1) {
-            self->acl = acl_get_fd(fdval);
+            new = acl_get_fd(fdval);
         }
     } else if(thesrc != NULL)
-        self->acl = acl_dup(thesrc->acl);
+        new = acl_dup(thesrc->acl);
     else if(filedef != NULL) {
         char *path = PyBytes_AS_STRING(filedef);
-        self->acl = acl_get_file(path, ACL_TYPE_DEFAULT);
+        new = acl_get_file(path, ACL_TYPE_DEFAULT);
         Py_DECREF(path);
     }
 #ifdef HAVE_LINUX
     else if(mode != -1)
-        self->acl = acl_from_mode(mode);
+        new = acl_from_mode(mode);
 #endif
 #ifdef HAVE_ACL_COPY_EXT
     else if(buf != NULL) {
-      if((self->acl = acl_copy_int(buf)) == NULL) {
-        PyErr_SetFromErrno(PyExc_IOError);
-        return -1;
-      }
+      new = acl_copy_int(buf);
     }
 #endif
     else
-        self->acl = acl_init(0);
+        new = acl_init(0);
 
-    if(self->acl == NULL) {
+    if(new == NULL) {
         PyErr_SetFromErrno(PyExc_IOError);
         return -1;
     }
 
+    /* Free the old acl_t without checking for error, we don't
+     * care right now */
+    if(self->acl != NULL)
+        acl_free(self->acl);
+
+    self->acl = new;
+
     return 0;
 }
 
diff --git a/tests/test_acls.py b/tests/test_acls.py
index 28f55c9..cc9a2e2 100644
--- a/tests/test_acls.py
+++ b/tests/test_acls.py
@@ -313,6 +313,18 @@ class TestLoad:
         assert acl1.valid()
         acl1.__init__(text=BASIC_ACL_TEXT) # type: ignore
         assert acl1.valid()
+        acl2 = ACL(mode=0o755)
+        assert acl1 != acl2
+        acl1.__init__(acl=acl2)  # type: ignore
+        assert acl1 == acl2
+
+    def test_entry_reinit_failure_noop(self):
+        a = posix1e.ACL(mode=0o0755)
+        b = posix1e.ACL(acl=a)
+        assert a == b
+        with pytest.raises(IOError):
+            a.__init__(text='foobar')
+        assert a == b
 
     @pytest.mark.xfail(reason="Unreliable test, re-init doesn't always invalidate children")
     def test_double_init_breaks_children(self):
-- 
2.39.2