audit: don't use GValue for tracking values in AuditField struct

When using a GValue, we really should call g_value_unset(). Otherwise
it is a code smell, even if we technically only created GValue with
static strings and integers.

But changing that is not easy, because the AuditField structs are
allocated on the stack, and in different functions. So we cannot just
pass a GDestroyNotify to GPtrArray to cleanup all those fields, because
by then they will be out of scope.

The proper solution would be to heap allocate the AuditField struct, add
them to the GPtrArray, and free them with the free function. But that
seams really unnecessary overhead, for something that is correct in
practice. Let's accept the fact that by the time the fields array gets
destroyed, it contains dangling pointers.

If we already embrace the dangling pointers and that stuff is allocated
on the stack and that we don't need to free, also get rid of GValue
and use our plain NMValueType and NMValueTypUnion. GValue really doesn't
give us much here. And it only makes us wonder: is it OK to not call
g_value_unset()? With the plain tracking of the values, we know that
it is OK.

1 files changed, 59 insertions, 36 deletions

diff --git a/src/core/nm-audit-manager.c b/src/core/nm-audit-manager.c
index 7517a1ab9a..44ba0e12bb 100644
--- a/src/core/nm-audit-manager.c
+++ b/src/core/nm-audit-manager.c
@@ -11,15 +11,18 @@
     #include <libaudit.h>
 #endif
 
-#include "libnm-glib-aux/nm-str-buf.h"
+#define NM_VALUE_TYPE_DEFINE_FUNCTIONS
+
 #include "libnm-core-aux-intern/nm-auth-subject.h"
+#include "libnm-glib-aux/nm-str-buf.h"
+#include "libnm-glib-aux/nm-value-type.h"
 #include "nm-config.h"
 #include "nm-dbus-manager.h"
 #include "settings/nm-settings-connection.h"
 
 /*****************************************************************************/
 
-typedef enum {
+typedef enum _nm_packed {
     BACKEND_LOG    = (1 << 0),
     BACKEND_AUDITD = (1 << 1),
     _BACKEND_LAST,
@@ -27,10 +30,11 @@ typedef enum {
 } AuditBackend;
 
 typedef struct {
-    const char * name;
-    GValue       value;
-    gboolean     need_encoding;
-    AuditBackend backends;
+    const char *    name;
+    AuditBackend    backends;
+    bool            need_encoding;
+    NMValueType     value_type;
+    NMValueTypUnion value;
 } AuditField;
 
 /*****************************************************************************/
@@ -86,20 +90,24 @@ _audit_field_init_string(AuditField * field,
                          gboolean     need_encoding,
                          AuditBackend backends)
 {
-    field->name          = name;
-    field->need_encoding = need_encoding;
-    field->backends      = backends;
-    g_value_init(&field->value, G_TYPE_STRING);
-    g_value_set_static_string(&field->value, str);
+    *field = (AuditField){
+        .name           = name,
+        .need_encoding  = need_encoding,
+        .backends       = backends,
+        .value_type     = NM_VALUE_TYPE_STRING,
+        .value.v_string = str,
+    };
 }
 
 static void
-_audit_field_init_uint(AuditField *field, const char *name, uint val, AuditBackend backends)
+_audit_field_init_uint64(AuditField *field, const char *name, guint64 val, AuditBackend backends)
 {
-    field->name     = name;
-    field->backends = backends;
-    g_value_init(&field->value, G_TYPE_UINT);
-    g_value_set_uint(&field->value, val);
+    *field = (AuditField){
+        .name           = name,
+        .backends       = backends,
+        .value_type     = NM_VALUE_TYPE_UINT64,
+        .value.v_uint64 = val,
+    };
 }
 
 static char *
@@ -109,15 +117,15 @@ build_message(GPtrArray *fields, AuditBackend backend)
     guint    i;
 
     for (i = 0; i < fields->len; i++) {
-        AuditField *field = fields->pdata[i];
+        const AuditField *field = fields->pdata[i];
 
         if (!NM_FLAGS_ANY(field->backends, backend))
             continue;
 
         nm_str_buf_append_required_delimiter(&strbuf, ' ');
 
-        if (G_VALUE_HOLDS_STRING(&field->value)) {
-            const char *str = g_value_get_string(&field->value);
+        if (field->value_type == NM_VALUE_TYPE_STRING) {
+            const char *str = field->value.v_string;
 
 #if HAVE_LIBAUDIT
             if (backend == BACKEND_AUDITD) {
@@ -131,15 +139,22 @@ build_message(GPtrArray *fields, AuditBackend backend)
                 continue;
             }
 #endif /* HAVE_LIBAUDIT */
+
             nm_str_buf_append_printf(&strbuf, "%s=\"%s\"", field->name, str);
-        } else if (G_VALUE_HOLDS_UINT(&field->value)) {
+            continue;
+        }
+
+        if (field->value_type == NM_VALUE_TYPE_UINT64) {
             nm_str_buf_append_printf(&strbuf,
-                                     "%s=%u",
+                                     "%s=%" G_GUINT64_FORMAT,
                                      field->name,
-                                     g_value_get_uint(&field->value));
-        } else
-            g_assert_not_reached();
+                                     field->value.v_uint64);
+            continue;
+        }
+
+        g_return_val_if_reached(NULL);
     }
+
     return nm_str_buf_finalize(&strbuf, NULL);
 }
 
@@ -197,9 +212,13 @@ _audit_log_helper(NMAuditManager *self,
                   gpointer        subject_context,
                   const char *    reason)
 {
-    AuditField      op_field = {}, pid_field = {}, uid_field = {};
-    AuditField      result_field = {}, reason_field = {};
-    gulong          pid, uid;
+    AuditField      op_field;
+    AuditField      pid_field;
+    AuditField      uid_field;
+    AuditField      result_field;
+    AuditField      reason_field;
+    gulong          pid;
+    gulong          uid;
     NMAuthSubject * subject                     = NULL;
     gs_unref_object NMAuthSubject *subject_free = NULL;
 
@@ -220,11 +239,11 @@ _audit_log_helper(NMAuditManager *self,
         pid = nm_auth_subject_get_unix_process_pid(subject);
         uid = nm_auth_subject_get_unix_process_uid(subject);
         if (pid != G_MAXULONG) {
-            _audit_field_init_uint(&pid_field, "pid", pid, BACKEND_ALL);
+            _audit_field_init_uint64(&pid_field, "pid", pid, BACKEND_ALL);
             g_ptr_array_add(fields, &pid_field);
         }
         if (uid != G_MAXULONG) {
-            _audit_field_init_uint(&uid_field, "uid", uid, BACKEND_ALL);
+            _audit_field_init_uint64(&uid_field, "uid", uid, BACKEND_ALL);
             g_ptr_array_add(fields, &uid_field);
         }
     }
@@ -269,8 +288,10 @@ _nm_audit_manager_log_connection_op(NMAuditManager *      self,
                                     gpointer              subject_context,
                                     const char *          reason)
 {
-    gs_unref_ptrarray GPtrArray *fields     = NULL;
-    AuditField                   uuid_field = {}, name_field = {}, args_field = {};
+    gs_unref_ptrarray GPtrArray *fields = NULL;
+    AuditField                   uuid_field;
+    AuditField                   name_field;
+    AuditField                   args_field;
 
     g_return_if_fail(op);
 
@@ -311,8 +332,8 @@ _nm_audit_manager_log_generic_op(NMAuditManager *self,
                                  gpointer        subject_context,
                                  const char *    reason)
 {
-    gs_unref_ptrarray GPtrArray *fields    = NULL;
-    AuditField                   arg_field = {};
+    gs_unref_ptrarray GPtrArray *fields = NULL;
+    AuditField                   arg_field;
 
     g_return_if_fail(op);
     g_return_if_fail(arg);
@@ -337,8 +358,10 @@ _nm_audit_manager_log_device_op(NMAuditManager *self,
                                 gpointer        subject_context,
                                 const char *    reason)
 {
-    gs_unref_ptrarray GPtrArray *fields          = NULL;
-    AuditField                   interface_field = {}, ifindex_field = {}, args_field = {};
+    gs_unref_ptrarray GPtrArray *fields = NULL;
+    AuditField                   interface_field;
+    AuditField                   ifindex_field;
+    AuditField                   args_field;
     int                          ifindex;
 
     g_return_if_fail(op);
@@ -355,7 +378,7 @@ _nm_audit_manager_log_device_op(NMAuditManager *self,
 
     ifindex = nm_device_get_ip_ifindex(device);
     if (ifindex > 0) {
-        _audit_field_init_uint(&ifindex_field, "ifindex", ifindex, BACKEND_ALL);
+        _audit_field_init_uint64(&ifindex_field, "ifindex", ifindex, BACKEND_ALL);
         g_ptr_array_add(fields, &ifindex_field);
     }