Merge Improvements to the SWIG binding handling of aa_log_record and %exception memory management

This patchset adds annotations so that SWIG can automatically manage the memory lifetimes of aa_log_record objects, and ensures proper cleanup is done in the %exception handler.

This is the first of a sequence of MRs to overhaul the SWIG bindings and fix pieces that never actually worked in the first place. As fixing those other pieces will require breaking changes, I am separating out the non-breaking changes into separate MRs.

Signed-off-by: Ryan Lee <ryan.lee@canonical.com>

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1334
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Ryan Lee <rlee287@yahoo.com>
This commit is contained in:
Ryan Lee 2024-10-01 18:55:24 +00:00
commit bcab725670
2 changed files with 52 additions and 1 deletions

View file

@ -8,6 +8,53 @@
%}
%include "typemaps.i"
%newobject parse_record;
%delobject free_record;
/*
* Despite its name, %delobject does not hook up destructors to language
* deletion mechanisms. Instead, it sets flags so that manually calling the
* free function and then deleting by language mechanisms doesn't cause a
* double-free.
*
* Additionally, we can manually extend the struct with a C++-like
* destructor. This ensures that the record struct is freed
* automatically when the high-level object goes out of scope.
*/
%extend aa_log_record {
~aa_log_record() {
free_record($self);
}
}
/*
* Generate a no-op free_record wrapper to avoid making a double-free footgun.
* Use rename directive to avoid colliding with the actual free_record, which
* we use above to clean up when the higher-level language deletes the object.
*
* Ideally we would not expose a free_record at all, but we need to maintain
* backwards compatibility with the existing high-level code that uses it.
*/
%rename(free_record) noop_free_record;
#ifdef SWIGPYTHON
%pythonprepend noop_free_record %{
import warnings
warnings.warn("free_record is now a no-op as the record's memory is handled automatically", DeprecationWarning)
%}
#endif
%feature("autodoc",
"This function used to free aa_log_record objects. Freeing is now handled "
"automatically, so this no-op function remains for backwards compatibility.") noop_free_record;
%inline %{
void noop_free_record(aa_log_record *record) {(void) record;}
%}
/*
* Do not autogenerate a wrapper around free_record. This does not prevent us
* from calling it ourselves in %extend C code.
*/
%ignore free_record;
%include <aalogparse.h>
/**
@ -31,8 +78,9 @@ extern int _aa_is_blacklisted(const char *name);
%exception {
$action
if (result < 0) {
// Unfortunately SWIG_exception does not support OSError
PyErr_SetFromErrno(PyExc_OSError);
return NULL;
SWIG_fail;
}
}
#endif

View file

@ -118,6 +118,9 @@ class AAPythonBindingsTests(unittest.TestCase):
# FIXME: out files should report log version?
# FIXME: or can we just deprecate v1 logs?
continue
elif key == "thisown":
# SWIG generates this key to track memory allocation
continue
elif key in NO_VALUE_MAP:
if NO_VALUE_MAP[key] == value:
continue