API: update netplan_delete_connection() to avoid spawning another process

Makefile

@@ -4,7 +4,7 @@ BUILDFLAGS = \
 	-g \
 	-fPIC \
 	-std=c99 \
-	-D_XOPEN_SOURCE=700 \
+	-D_GNU_SOURCE \
 	-DSBINDIR=\"$(SBINDIR)\" \
 	-I${CURDIR}/include \
 	-Wall \

abi-compat/suppressions.abignore

@@ -7,3 +7,8 @@
 [suppress_variable]
   name = global_state
   type_name = NetplanState
+
+# TODO: drop after 0.106
+[suppress_function]
+  name = find_yaml_glob
+  parameter = '1 glob_t*

meson.build

@@ -30,7 +30,7 @@ find = find_program('find')
 
 add_project_arguments(
     '-DSBINDIR="' + join_paths(get_option('prefix'), get_option('sbindir')) + '"',
-    '-D_XOPEN_SOURCE=700',
+    '-D_GNU_SOURCE',
     language: 'c')
 
 inc = include_directories('include')

src/util-internal.h

@@ -17,7 +17,6 @@
 
 #pragma once
 
-#define __USE_MISC
 #include <glob.h>
 #include <glib.h>
 #include "types-internal.h"

src/util.c

@@ -21,6 +21,7 @@
 #include <fnmatch.h>
 #include <errno.h>
 #include <string.h>
+#include <sys/mman.h>
 
 #include <glib.h>
 #include <glib/gprintf.h>
@@ -40,6 +41,8 @@ wifi_frequency_24;
 NETPLAN_ABI GHashTable*
 wifi_frequency_5;
 
+const gchar* FALLBACK_FILENAME = "70-netplan-set.yaml";
+
 typedef struct netplan_state_iterator RealStateIter;
 /**
  * Create the parent directories of given file path. Exit program on failure.
@@ -487,55 +490,93 @@ systemd_escape(char* string)
 gboolean
 netplan_delete_connection(const char* id, const char* rootdir)
 {
-    g_autofree gchar* del = NULL;
+    g_autofree gchar* yaml_path = NULL;
     g_autoptr(GError) error = NULL;
     NetplanNetDefinition* nd = NULL;
     gboolean ret = TRUE;
+    int patch_fd = -1;
 
-    NetplanState* np_state = netplan_state_new();
-    NetplanParser* npp = netplan_parser_new();
+    NetplanParser* input_parser = netplan_parser_new();
+    NetplanState* input_state = netplan_state_new();
+    NetplanParser* output_parser = NULL;
+    NetplanState* output_state = NULL;
 
     /* parse all YAML files */
-    if (   !netplan_parser_load_yaml_hierarchy(npp, rootdir, &error)
-        || !netplan_state_import_parser_results(np_state, npp, &error)) {
+    if (   !netplan_parser_load_yaml_hierarchy(input_parser, rootdir, &error)
+        || !netplan_state_import_parser_results(input_state, input_parser, &error)) {
+        g_fprintf(stderr, "netplan_delete_connection: Cannot parse input: %s\n", error->message);
+        ret = FALSE;
+        goto cleanup;
+    }
+
+    /* find specified netdef in input state */
+    nd = netplan_state_get_netdef(input_state, id);
+    if (!nd) {
+        g_fprintf(stderr, "netplan_delete_connection: Cannot delete %s, does not exist.\n", id);
+        ret = FALSE;
+        goto cleanup;
+    }
+
+    /* Build up a tab-separated YAML path for this Netdef (e.g. network.ethernets.eth0=...) */
+    yaml_path = g_strdup_printf("network\t%s\t%s", netplan_def_type_name(nd->type), id);
+
+    /* create a temporary file in memory, to hold our YAML patch */
+    patch_fd = memfd_create("patch.yaml", 0);
+    if (patch_fd < 0) {
         // LCOV_EXCL_START
-        g_fprintf(stderr, "%s\n", error->message);
+        g_fprintf(stderr, "netplan_delete_connection: Cannot create memfd: %m\n");
         ret = FALSE;
         goto cleanup;
         // LCOV_EXCL_STOP
     }
-
-    if (!np_state->netdefs) {
+    if (!netplan_util_create_yaml_patch(yaml_path, "NULL", patch_fd, &error)) {
         // LCOV_EXCL_START
-        g_fprintf(stderr, "netplan_delete_connection: %s\n", error->message);
+        g_fprintf(stderr, "netplan_delete_connection: Cannot create YAML patch: %s\n", error->message);
         ret = FALSE;
         goto cleanup;
         // LCOV_EXCL_STOP
     }
 
-    /* find filename for specified netdef ID */
-    nd = g_hash_table_lookup(np_state->netdefs, id);
-    if (!nd) {
-        g_warning("netplan_delete_connection: Cannot delete %s, does not exist.", id);
+    /* Create a new parser & state to hold our output YAML, ignoring the to be
+     * deleted Netdef from the patch */
+    output_parser = netplan_parser_new();
+    output_state = netplan_state_new();
+
+    lseek(patch_fd, 0, SEEK_SET);
+    if (   !netplan_parser_load_nullable_fields(output_parser, patch_fd, &error)
+        || !netplan_parser_load_yaml_hierarchy(output_parser, rootdir, &error)) {
+        // LCOV_EXCL_START
+        g_fprintf(stderr, "netplan_delete_connection: Cannot load output state: %s\n", error->message);
         ret = FALSE;
         goto cleanup;
+        // LCOV_EXCL_STOP
     }
 
-    del = g_strdup_printf("network.%s.%s=NULL", netplan_def_type_name(nd->type), id);
+    lseek(patch_fd, 0, SEEK_SET);
+    if (!netplan_parser_load_yaml_from_fd(output_parser, patch_fd, &error)) {
+        // LCOV_EXCL_START
+        g_fprintf(stderr, "netplan_delete_connection: Cannot parse YAML patch: %s\n", error->message);
+        ret = FALSE;
+        goto cleanup;
+        // LCOV_EXCL_STOP
+    }
 
-    /* TODO: refactor logic to actually be inside the library instead of spawning another process */
-    const gchar *argv[] = { SBINDIR "/" "netplan", "set", del, NULL, NULL, NULL };
-    if (rootdir) {
-        argv[3] = "--root-dir";
-        argv[4] = rootdir;
+    /* We're only deleting some data, so FALLBACK_FILENAME should never be created */
+    if (   !netplan_state_import_parser_results(output_state, output_parser, &error)
+        || !netplan_state_update_yaml_hierarchy(output_state, FALLBACK_FILENAME, rootdir, &error)) {
+        // LCOV_EXCL_START
+        g_fprintf(stderr, "netplan_delete_connection: Cannot write output state: %s\n", error->message);
+        ret = FALSE;
+        goto cleanup;
+        // LCOV_EXCL_STOP
     }
-    if (getenv("TEST_NETPLAN_CMD") != 0)
-       argv[0] = getenv("TEST_NETPLAN_CMD");
-    ret = g_spawn_sync(NULL, (gchar**)argv, NULL, 0, NULL, NULL, NULL, NULL, NULL, NULL);
 
 cleanup:
-    if (npp) netplan_parser_clear(&npp);
-    if (np_state) netplan_state_clear(&np_state);
+    if (input_parser) netplan_parser_clear(&input_parser);
+    if (input_state) netplan_state_clear(&input_state);
+    if (output_parser) netplan_parser_clear(&output_parser);
+    if (output_state) netplan_state_clear(&output_state);
+    if (patch_fd >= 0) close(patch_fd);
     return ret;
 }
 

tests/ctests/meson.build

@@ -15,7 +15,11 @@ foreach name, should_fail: tests
     '@[email protected]'.format(name),
     include_directories: [inc, inc_tests],
     dependencies: [cmocka, glib, gio, yaml, uuid],
-    c_args: ['-Wno-deprecated-declarations', '-DFIXTURESDIR="' + meson.project_source_root() + '/tests/ctests/fixtures"'],
+    c_args: [
+      '-DFIXTURESDIR="' + meson.project_source_root() + '/tests/ctests/fixtures"',
+      '-Wno-deprecated-declarations',
+      '-D_GNU_SOURCE',
+      ],
     )
   test(name, exe, should_fail: should_fail)
 endforeach

tests/ctests/test_netplan_deprecated.c

@@ -10,7 +10,6 @@
 #include "types-internal.h"
 #include "types.h"
 
-#undef __USE_MISC
 #include "error.c"
 #include "names.c"
 #include "validation.c"

tests/ctests/test_netplan_error.c

@@ -8,9 +8,9 @@
 #include "netplan.h"
 #include "parse.h"
 
-#undef __USE_MISC
 #include "error.c"
 #include "names.c"
+#include "netplan.c"
 #include "validation.c"
 #include "types.c"
 #include "util.c"

tests/ctests/test_netplan_misc.c

@@ -10,7 +10,6 @@
 #include "types-internal.h"
 #include "types.h"
 
-#undef __USE_MISC
 #include "error.c"
 #include "names.c"
 #include "validation.c"

tests/ctests/test_netplan_parser.c

@@ -10,9 +10,9 @@
 #include "parse.h"
 #include "util.h"
 
-#undef __USE_MISC
 #include "error.c"
 #include "names.c"
+#include "netplan.c"
 #include "validation.c"
 #include "types.c"
 #include "util.c"

tests/ctests/test_netplan_state.c

@@ -8,9 +8,9 @@
 #include "netplan.h"
 #include "parse.h"
 
-#undef __USE_MISC
 #include "error.c"
 #include "names.c"
+#include "netplan.c"
 #include "validation.c"
 #include "types.c"
 #include "util.c"

tests/ctests/test_netplan_validation.c

@@ -8,9 +8,9 @@
 #include "netplan.h"
 #include "parse.h"
 
-#undef __USE_MISC
 #include "error.c"
 #include "names.c"
+#include "netplan.c"
 #include "validation.c"
 #include "types.c"
 #include "util.c"

tests/test_libnetplan.py

@@ -28,6 +28,7 @@
 from tests.test_utils import MockCmd
 
 from utils import state_from_yaml
+from netplan.cli.commands.set import FALLBACK_FILENAME
 
 import netplan.libnetplan as libnetplan
 
@@ -100,6 +101,7 @@ def test_delete_connection(self):
         # Parse all YAML and delete 'some-netplan-id' connection file
         self.assertTrue(lib.netplan_delete_connection('some-netplan-id'.encode(), self.workdir.name.encode()))
         self.assertFalse(os.path.isfile(orig))
+        self.assertFalse(os.path.isfile(os.path.join(self.confdir, FALLBACK_FILENAME)))
 
     def test_delete_connection_id_not_found(self):
         orig = os.path.join(self.confdir, 'some-filename.yaml')
@@ -132,6 +134,16 @@ def test_delete_connection_two_in_file(self):
         with open(orig, 'r') as f:
             self.assertEqual(f.read(), 'network:\n  version: 2\n  ethernets:\n    other-id:\n      dhcp6: true\n')
 
+    def test_delete_connection_invalid(self):
+        orig = os.path.join(self.confdir, 'some-filename.yaml')
+        with open(orig, 'w') as f:
+            f.write('INVALID')
+        self.assertTrue(os.path.isfile(orig))
+        with capture_stderr() as outf:
+            self.assertFalse(lib.netplan_delete_connection('some-netplan-id'.encode(), self.workdir.name.encode()))
+            with open(outf.name, 'r') as f:
+                self.assertIn('Cannot parse input', f.read())
+
     def test_write_netplan_conf(self):
         netdef_id = 'some-netplan-id'
         orig = os.path.join(self.confdir, 'some-filename.yaml')