From 65e6c78eb9338c8b3ac77b89880ca4f5f85d62b5 Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@k1024.org>
Date: Fri, 29 Nov 2019 15:42:42 +0100
Subject: [PATCH] Switch ACL to be always-initialised

This is the last object to change, but the semantics here are more
complex. Since the ACL doesn't have a parent, and the init signature
is complex, we can't detect "same-reinit", we allow arbitrary-reinit,
but this makes existing live entries be undefined; they might point to
a different entry in the new ACL, or not be valid, etc.

It could be possible to prevent re-init, but doing so requires
trickery which might be broken by serialisation, so let's just leave
it there and document it as such.
---
 NEWS               |  4 +++-
 acl.c              | 17 +++++++++++++----
 tests/test_acls.py | 18 ++++++++++++++++++
 3 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index 0105cdc..466481f 100644
--- a/NEWS
+++ b/NEWS
@@ -22,7 +22,9 @@ Important API changes/bug fixes:
   to prevent the need for checking initialisation status in all code
   paths; this also (implicitly) fixes memory leaks on re-initialisation
   (calling `__init__(…)` on an existing object) and segfaults (!) on
-  non-initialised object attribute access.
+  non-initialised object attribute access. Note ACL re-initialisation is
+  tricky and (still) leads to undefined behaviour of existing Entry
+  objects pointing to it.
 - 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 e59467d..0372dc6 100644
--- a/acl.c
+++ b/acl.c
@@ -104,15 +104,24 @@ typedef struct {
 static PyObject* ACL_new(PyTypeObject* type, PyObject* args,
                          PyObject *keywds) {
     PyObject* newacl;
+    ACL_Object *acl;
 
     newacl = type->tp_alloc(type, 0);
 
-    if(newacl != NULL) {
-        ((ACL_Object*)newacl)->acl = NULL;
+    if(newacl == NULL) {
+        return NULL;
+    }
+    acl = (ACL_Object*) newacl;
+
+    acl->acl = acl_init(0);
+    if (acl->acl == NULL) {
+        PyErr_SetFromErrno(PyExc_IOError);
+        Py_DECREF(newacl);
+        return NULL;
+    }
 #ifdef HAVEL_LEVEL2
-        ((ACL_Object*)newacl)->entry_id = ACL_FIRST_ENTRY;
+    acl->entry_id = ACL_FIRST_ENTRY;
 #endif
-    }
 
     return newacl;
 }
diff --git a/tests/test_acls.py b/tests/test_acls.py
index 16ebd78..710a129 100644
--- a/tests/test_acls.py
+++ b/tests/test_acls.py
@@ -294,12 +294,30 @@ class TestLoad:
         with pytest.raises(TypeError):
           posix1e.ACL(foo="bar")
 
+    def test_uninit(self):
+        """Checks that uninit is actually empty init"""
+        acl = posix1e.ACL.__new__(posix1e.ACL)
+        assert not acl.valid()
+        e = acl.append()
+        e.permset
+        acl.delete_entry(e)
+
     def test_double_init(self):
         acl1 = posix1e.ACL(text=BASIC_ACL_TEXT)
         assert acl1.valid()
         acl1.__init__(text=BASIC_ACL_TEXT)
         assert acl1.valid()
 
+    @pytest.mark.xfail(reason="Unreliable test, re-init doesn't always invalidate children")
+    def test_double_init_breaks_children(self):
+        acl = posix1e.ACL()
+        e = acl.append()
+        e.permset.write = True
+        acl.__init__()
+        with pytest.raises(EnvironmentError):
+            e.permset.write = False
+
+
 class TestAclExtensions:
     """ACL extensions checks"""
 
-- 
2.39.2