From 07791bca7766bb6da1caad4a80c068fef53adefa Mon Sep 17 00:00:00 2001 From: Iustin Pop Date: Sat, 22 Apr 2023 02:20:16 +0200 Subject: [PATCH] Rework how acl __setstate__ handles some errors This resolves the very old comment 'Should we ignore errors'. Upon more thinking, yes, ignoring errors is better, and the way the code was, it contained a memory leak (on a very unlikely path). So rework it to ignore errors in freeing the old ACL, since we don't care about the old ACL, and the new one is successfully allocated. --- acl.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/acl.c b/acl.c index 1be7009..ffb6d6b 100644 --- a/acl.c +++ b/acl.c @@ -558,10 +558,13 @@ static PyObject* ACL_set_state(PyObject *obj, PyObject* args) { if((ptr = acl_copy_int(buf)) == NULL) return PyErr_SetFromErrno(PyExc_IOError); - /* Free the old acl. Should we ignore errors here? */ if(self->acl != NULL) { - if(acl_free(self->acl) == -1) - return PyErr_SetFromErrno(PyExc_IOError); /* LCOV_EXCL_LINE */ + /* Ignore errors in freeing the previous acl. We already + allocated the new acl, and the state of the previous one is + suspect if freeing failed (in Linux's libacl, deallocating + a valid ACL can't actually happen, so this path is + unlikely. */ + acl_free(self->acl); /* LCOV_EXCL_LINE */ } self->acl = ptr; -- 2.39.5