lib: unify how constant names are exposed and used

This patch moves all the various enum-to-string arrays into a single compilation unit instead of exposing them directly into headers, and exposes the data through simple functions. This approach has multiple advantages: * If we want to expose one or several of those conversion functions to the ABI, we now can add new constants without breaking the ABI, as the functions can deal with unknown constants in a sensible manner. Directly exposing the arrays in the headers as was previously done was a recipy for disaster, as the array is baked into the compiled client code, and will not be resized if we decide to add new values to the enums in new versions of the library. * It's marginally faster to compile, as the compiler doesn't have to copy around multiple times the same arrays, only to deduplicate them at link time (if it even does that). Note that the naming scheme for the new functions differs from all the previous API, using the `np_` prefix. This is because this patch is one of the first parts of a broader effort to create an actual library experience around libnetplan, which will need to basically change the whole current API. Using a new prefix allows me to quickly see if a symbol is from the legacy ABI or the new API/ABI. I'm using this scheme here as I'm anticipating that we'll want to expose them in our public API at some point (we already have internal consumers outside of the library, anyway).
canonical · Sep 24, 2021 · d950faa · d950faa
1 parent 17d3848
commit d950faa
Show file tree

Hide file tree

Showing 11 changed files with 141 additions and 89 deletions.
diff --git a/Makefile b/Makefile
@@ -38,14 +38,14 @@ default: netplan/_features.py generate netplan-dbus dbus/io.netplan.Netplan.serv
 %.o: src/%.c
 	$(CC) $(BUILDFLAGS) $(CFLAGS) $(LDFLAGS) -c $^ `pkg-config --cflags --libs glib-2.0 gio-2.0 yaml-0.1 uuid`
 
-libnetplan.so.$(NETPLAN_SOVER): parse.o netdef.o netplan.o util.o validation.o error.o parse-nm.o
+libnetplan.so.$(NETPLAN_SOVER): parse.o netdef.o netplan.o util.o validation.o error.o parse-nm.o names.o
 	$(CC) -shared -Wl,-soname,libnetplan.so.$(NETPLAN_SOVER) $(BUILDFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ $^ `pkg-config --libs glib-2.0 gio-2.0 yaml-0.1`
 	ln -snf libnetplan.so.$(NETPLAN_SOVER) libnetplan.so
 
-generate: libnetplan.so.$(NETPLAN_SOVER) nm.o networkd.o openvswitch.o generate.o sriov.o
+generate: libnetplan.so.$(NETPLAN_SOVER) nm.o networkd.o openvswitch.o generate.o sriov.o names.o
 	$(CC) $(BUILDFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ $^ -L. -lnetplan `pkg-config --cflags --libs glib-2.0 gio-2.0 yaml-0.1 uuid`
 
-netplan-dbus: src/dbus.c src/_features.h netdef.o parse.o util.o validation.o error.o
+netplan-dbus: src/dbus.c src/_features.h netdef.o parse.o util.o validation.o error.o names.o
 	$(CC) $(BUILDFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ $(patsubst %.h,,$^) `pkg-config --cflags --libs libsystemd glib-2.0 gio-2.0 yaml-0.1`
 
 src/_features.h: src/[^_]*.[hc]

diff --git a/src/generate.c b/src/generate.c
@@ -28,6 +28,7 @@
 
 #include "util.h"
 #include "parse.h"
+#include "names.h"
 #include "networkd.h"
 #include "nm.h"
 #include "openvswitch.h"
@@ -157,7 +158,7 @@ find_interface(gchar* interface)
          const NetplanNetDefinition *nd = (NetplanNetDefinition *)g_ptr_array_index (found, 0);
          g_printf("id=%s, backend=%s, set_name=%s, match_name=%s, match_mac=%s, match_driver=%s\n",
              nd->id,
-             netplan_backend_to_name[nd->backend],
+             np_backend_name(nd->backend),
              nd->set_name,
              nd->match.original_name,
              nd->match.mac,

diff --git a/src/names.c b/src/names.c
@@ -0,0 +1,89 @@
+#include <glib.h>
+
+#include "names.h"
+#include "parse.h"
+
+/* Non-static as we need it for ABI compatibility, see at the end of the file */
+const char* const np_backend_to_str[NETPLAN_BACKEND_MAX_] = {
+        [NETPLAN_BACKEND_NONE] = "none",
+        [NETPLAN_BACKEND_NETWORKD] = "networkd",
+        [NETPLAN_BACKEND_NM] = "NetworkManager",
+        [NETPLAN_BACKEND_OVS] = "OpenVSwitch",
+};
+
+static const char* const np_wifi_mode_to_str[NETPLAN_WIFI_MODE_MAX_] = {
+    [NETPLAN_WIFI_MODE_INFRASTRUCTURE] = "infrastructure",
+    [NETPLAN_WIFI_MODE_ADHOC] = "adhoc",
+    [NETPLAN_WIFI_MODE_AP] = "ap",
+    [NETPLAN_WIFI_MODE_OTHER] = NULL,
+};
+
+
+static const char* const np_def_type_to_str[NETPLAN_DEF_TYPE_MAX_] = {
+    [NETPLAN_DEF_TYPE_NONE] = NULL,
+    [NETPLAN_DEF_TYPE_ETHERNET] = "ethernets",
+    [NETPLAN_DEF_TYPE_WIFI] = "wifis",
+    [NETPLAN_DEF_TYPE_MODEM] = "modems",
+    [NETPLAN_DEF_TYPE_BRIDGE] = "bridges",
+    [NETPLAN_DEF_TYPE_BOND] = "bonds",
+    [NETPLAN_DEF_TYPE_VLAN] = "vlans",
+    [NETPLAN_DEF_TYPE_TUNNEL] = "tunnels",
+    [NETPLAN_DEF_TYPE_PORT] = NULL,
+    [NETPLAN_DEF_TYPE_NM] = "nm-devices",
+};
+
+static const char* const np_auth_key_management_type_to_str[NETPLAN_AUTH_KEY_MANAGEMENT_MAX] = {
+    [NETPLAN_AUTH_KEY_MANAGEMENT_NONE] = "none",
+    [NETPLAN_AUTH_KEY_MANAGEMENT_WPA_PSK] = "psk",
+    [NETPLAN_AUTH_KEY_MANAGEMENT_WPA_EAP] = "eap",
+    [NETPLAN_AUTH_KEY_MANAGEMENT_8021X] = "802.1x",
+};
+
+static const char* const np_auth_eap_method_to_str[NETPLAN_AUTH_EAP_METHOD_MAX] = {
+    [NETPLAN_AUTH_EAP_NONE] = NULL,
+    [NETPLAN_AUTH_EAP_TLS] = "tls",
+    [NETPLAN_AUTH_EAP_PEAP] = "peap",
+    [NETPLAN_AUTH_EAP_TTLS] = "ttls",
+};
+
+static const char* const np_tunnel_mode_to_str[NETPLAN_TUNNEL_MODE_MAX_] = {
+    [NETPLAN_TUNNEL_MODE_UNKNOWN] = NULL,
+    [NETPLAN_TUNNEL_MODE_IPIP] = "ipip",
+    [NETPLAN_TUNNEL_MODE_GRE] = "gre",
+    [NETPLAN_TUNNEL_MODE_SIT] = "sit",
+    [NETPLAN_TUNNEL_MODE_ISATAP] = "isatap",
+    [NETPLAN_TUNNEL_MODE_VTI] = "vti",
+    [NETPLAN_TUNNEL_MODE_IP6IP6] = "ip6ip6",
+    [NETPLAN_TUNNEL_MODE_IPIP6] = "ipip6",
+    [NETPLAN_TUNNEL_MODE_IP6GRE] = "ip6gre",
+    [NETPLAN_TUNNEL_MODE_VTI6] = "vti6",
+    [NETPLAN_TUNNEL_MODE_GRETAP] = "gretap",
+    [NETPLAN_TUNNEL_MODE_IP6GRETAP] = "ip6gretap",
+    [NETPLAN_TUNNEL_MODE_WIREGUARD] = "wireguard",
+};
+
+static const char* const np_addr_gen_mode_to_str[NETPLAN_ADDRGEN_MAX] = {
+    [NETPLAN_ADDRGEN_DEFAULT] = NULL,
+    [NETPLAN_ADDRGEN_EUI64] = "eui64",
+    [NETPLAN_ADDRGEN_STABLEPRIVACY] = "stable-privacy"
+};
+
+#define NAME_FUNCTION(_radical, _type) const char *np_ ## _radical ## _name( _type val) \
+{ \
+    return (val < sizeof( np_ ## _radical ## _to_str )) ?  np_ ## _radical ## _to_str [val] : NULL; \
+}
+
+NAME_FUNCTION(backend, NetplanBackend);
+NAME_FUNCTION(def_type, NetplanDefType);
+NAME_FUNCTION(auth_key_management_type, NetplanAuthKeyManagementType);
+NAME_FUNCTION(auth_eap_method, NetplanAuthEAPMethod);
+NAME_FUNCTION(tunnel_mode, NetplanTunnelMode);
+NAME_FUNCTION(addr_gen_mode, NetplanAddrGenMode);
+NAME_FUNCTION(wifi_mode, NetplanWifiMode);
+
+/* ABI compatibility definitions */
+
+const char *
+tunnel_mode_to_string(NetplanTunnelMode val) __attribute__ ((alias ("np_tunnel_mode_name")));
+
+extern const char* netplan_backend_to_name __attribute__((alias("np_backend_to_str")));
diff --git a/src/names.h b/src/names.h
@@ -0,0 +1,24 @@
+#pragma once
+
+#include "netplan.h"
+
+const char*
+np_backend_name(NetplanBackend val);
+
+const char*
+np_def_type_name(NetplanDefType val);
+
+const char*
+np_auth_key_management_type_name(NetplanAuthKeyManagementType val);
+
+const char*
+np_auth_eap_method_name(NetplanAuthEAPMethod val);
+
+const char*
+np_tunnel_mode_name(NetplanTunnelMode val);
+
+const char*
+np_addr_gen_mode_name(NetplanAddrGenMode val);
+
+const char*
+np_wifi_mode_name(NetplanWifiMode val);
diff --git a/src/netplan.c b/src/netplan.c
@@ -20,6 +20,7 @@
 
 #include "netplan.h"
 #include "parse.h"
+#include "names.h"
 
 gchar *tmp = NULL;
 
@@ -41,8 +42,8 @@ write_auth(yaml_event_t* event, yaml_emitter_t* emitter, NetplanAuthenticationSe
 {
     YAML_SCALAR_PLAIN(event, emitter, "auth");
     YAML_MAPPING_OPEN(event, emitter);
-    YAML_STRING(event, emitter, "key-management", netplan_auth_key_management_type_to_str[auth.key_management]);
-    YAML_STRING(event, emitter, "method", netplan_auth_eap_method_to_str[auth.eap_method]);
+    YAML_STRING(event, emitter, "key-management", np_auth_key_management_type_name(auth.key_management));
+    YAML_STRING(event, emitter, "method", np_auth_eap_method_name(auth.eap_method));
     YAML_STRING(event, emitter, "anonymous-identity", auth.anonymous_identity);
     YAML_STRING(event, emitter, "identity", auth.identity);
     YAML_STRING(event, emitter, "ca-certificate", auth.ca_certificate);
@@ -254,7 +255,7 @@ write_access_points(yaml_event_t* event, yaml_emitter_t* emitter, const NetplanN
         if (ap->has_auth)
             write_auth(event, emitter, ap->auth);
         if (ap->mode != NETPLAN_WIFI_MODE_INFRASTRUCTURE)
-            YAML_STRING(event, emitter, "mode", netplan_wifi_mode_to_str[ap->mode]);
+            YAML_STRING(event, emitter, "mode", np_wifi_mode_name(ap->mode));
         if (!write_backend_settings(event, emitter, ap->backend_settings)) goto error;
         YAML_MAPPING_CLOSE(event, emitter);
     }
@@ -367,7 +368,7 @@ error: return FALSE; // LCOV_EXCL_LINE
 static gboolean
 write_tunnel_settings(yaml_event_t* event, yaml_emitter_t* emitter, const NetplanNetDefinition* def)
 {
-    YAML_STRING(event, emitter, "mode", netplan_tunnel_mode_to_str[def->tunnel.mode]);
+    YAML_STRING(event, emitter, "mode", np_tunnel_mode_name(def->tunnel.mode));
     YAML_STRING(event, emitter, "local", def->tunnel.local_ip);
     YAML_STRING(event, emitter, "remote", def->tunnel.remote_ip);
     if (def->tunnel.fwmark)
@@ -651,7 +652,7 @@ _serialize_yaml(yaml_event_t* event, yaml_emitter_t* emitter, const NetplanNetDe
 
     YAML_STRING(event, emitter, "macaddress", def->set_mac);
     YAML_STRING(event, emitter, "set-name", def->set_name);
-    YAML_STRING(event, emitter, "ipv6-address-generation", netplan_addr_gen_mode_to_str[def->ip6_addr_gen_mode]);
+    YAML_STRING(event, emitter, "ipv6-address-generation", np_addr_gen_mode_name(def->ip6_addr_gen_mode));
     YAML_STRING(event, emitter, "ipv6-address-token", def->ip6_addr_gen_token);
     if (def->ip6_privacy)
         YAML_STRING_PLAIN(event, emitter, "ipv6-privacy", "true");
@@ -825,8 +826,8 @@ write_netplan_conf(const NetplanNetDefinition* def, const char* rootdir)
     YAML_MAPPING_OPEN(event, emitter);
     YAML_STRING_PLAIN(event, emitter, "version", "2");
 
-    if (netplan_def_type_to_str[def->type]) {
-        YAML_SCALAR_PLAIN(event, emitter, netplan_def_type_to_str[def->type]);
+    if (np_def_type_name(def->type)) {
+        YAML_SCALAR_PLAIN(event, emitter, np_def_type_name(def->type));
         YAML_MAPPING_OPEN(event, emitter);
         _serialize_yaml(event, emitter, def);
         YAML_MAPPING_CLOSE(event, emitter);
@@ -901,8 +902,8 @@ write_netplan_conf_full(const char* file_hint, const char* rootdir)
             for (unsigned i = 0; i < NETPLAN_DEF_TYPE_MAX_; ++i) {
                 /* Per-netdef config */
                 if (g_hash_table_find(netdefs, contains_netdef_type, &i)) {
-                    if (netplan_def_type_to_str[i]) {
-                        YAML_SCALAR_PLAIN(event, emitter, netplan_def_type_to_str[i]);
+                    if (np_def_type_name(i)) {
+                        YAML_SCALAR_PLAIN(event, emitter, np_def_type_name(i));
                         YAML_MAPPING_OPEN(event, emitter);
                         g_hash_table_iter_init(&iter, netdefs);
                         while (g_hash_table_iter_next (&iter, &key, &value)) {

diff --git a/src/netplan.h b/src/netplan.h
@@ -93,53 +93,4 @@
     yaml_emitter_delete(emitter_ptr); \
 }
 
-static const char* const netplan_def_type_to_str[NETPLAN_DEF_TYPE_MAX_] = {
-    [NETPLAN_DEF_TYPE_NONE] = NULL,
-    [NETPLAN_DEF_TYPE_ETHERNET] = "ethernets",
-    [NETPLAN_DEF_TYPE_WIFI] = "wifis",
-    [NETPLAN_DEF_TYPE_MODEM] = "modems",
-    [NETPLAN_DEF_TYPE_BRIDGE] = "bridges",
-    [NETPLAN_DEF_TYPE_BOND] = "bonds",
-    [NETPLAN_DEF_TYPE_VLAN] = "vlans",
-    [NETPLAN_DEF_TYPE_TUNNEL] = "tunnels",
-    [NETPLAN_DEF_TYPE_PORT] = NULL,
-    [NETPLAN_DEF_TYPE_NM] = "nm-devices",
-};
-
-static const char* const netplan_auth_key_management_type_to_str[NETPLAN_AUTH_KEY_MANAGEMENT_MAX] = {
-    [NETPLAN_AUTH_KEY_MANAGEMENT_NONE] = "none",
-    [NETPLAN_AUTH_KEY_MANAGEMENT_WPA_PSK] = "psk",
-    [NETPLAN_AUTH_KEY_MANAGEMENT_WPA_EAP] = "eap",
-    [NETPLAN_AUTH_KEY_MANAGEMENT_8021X] = "802.1x",
-};
-
-static const char* const netplan_auth_eap_method_to_str[NETPLAN_AUTH_EAP_METHOD_MAX] = {
-    [NETPLAN_AUTH_EAP_NONE] = NULL,
-    [NETPLAN_AUTH_EAP_TLS] = "tls",
-    [NETPLAN_AUTH_EAP_PEAP] = "peap",
-    [NETPLAN_AUTH_EAP_TTLS] = "ttls",
-};
-
-static const char* const netplan_tunnel_mode_to_str[NETPLAN_TUNNEL_MODE_MAX_] = {
-    [NETPLAN_TUNNEL_MODE_UNKNOWN] = NULL,
-    [NETPLAN_TUNNEL_MODE_IPIP] = "ipip",
-    [NETPLAN_TUNNEL_MODE_GRE] = "gre",
-    [NETPLAN_TUNNEL_MODE_SIT] = "sit",
-    [NETPLAN_TUNNEL_MODE_ISATAP] = "isatap",
-    [NETPLAN_TUNNEL_MODE_VTI] = "vti",
-    [NETPLAN_TUNNEL_MODE_IP6IP6] = "ip6ip6",
-    [NETPLAN_TUNNEL_MODE_IPIP6] = "ipip6",
-    [NETPLAN_TUNNEL_MODE_IP6GRE] = "ip6gre",
-    [NETPLAN_TUNNEL_MODE_VTI6] = "vti6",
-    [NETPLAN_TUNNEL_MODE_GRETAP] = "gretap",
-    [NETPLAN_TUNNEL_MODE_IP6GRETAP] = "ip6gretap",
-    [NETPLAN_TUNNEL_MODE_WIREGUARD] = "wireguard",
-};
-
-static const char* const netplan_addr_gen_mode_to_str[NETPLAN_ADDRGEN_MAX] = {
-    [NETPLAN_ADDRGEN_DEFAULT] = NULL,
-    [NETPLAN_ADDRGEN_EUI64] = "eui64",
-    [NETPLAN_ADDRGEN_STABLEPRIVACY] = "stable-privacy"
-};
-
 void write_netplan_conf(const NetplanNetDefinition* def, const char* rootdir);
diff --git a/src/networkd.c b/src/networkd.c
@@ -27,6 +27,7 @@
 
 #include "networkd.h"
 #include "parse.h"
+#include "names.h"
 #include "util.h"
 #include "validation.h"
 
@@ -139,7 +140,7 @@ write_tunnel_params(GString* s, const NetplanNetDefinition* def)
 
     g_string_printf(params, "Independent=true\n");
     if (def->tunnel.mode == NETPLAN_TUNNEL_MODE_IPIP6 || def->tunnel.mode == NETPLAN_TUNNEL_MODE_IP6IP6)
-        g_string_append_printf(params, "Mode=%s\n", tunnel_mode_to_string(def->tunnel.mode));
+        g_string_append_printf(params, "Mode=%s\n", np_tunnel_mode_name(def->tunnel.mode));
     g_string_append_printf(params, "Local=%s\n", def->tunnel.local_ip);
     g_string_append_printf(params, "Remote=%s\n", def->tunnel.remote_ip);
     if (def->tunnel_ttl)
@@ -390,7 +391,7 @@ write_netdev_file(const NetplanNetDefinition* def, const char* rootdir, const ch
                 case NETPLAN_TUNNEL_MODE_VTI6:
                 case NETPLAN_TUNNEL_MODE_WIREGUARD:
                     g_string_append_printf(s, "Kind=%s\n",
-                                           tunnel_mode_to_string(def->tunnel.mode));
+                                           np_tunnel_mode_name(def->tunnel.mode));
                     break;
 
                 case NETPLAN_TUNNEL_MODE_IP6IP6:

diff --git a/src/parse.c b/src/parse.c
@@ -29,9 +29,13 @@
 
 #include "parse.h"
 #include "util.h"
+#include "names.h"
 #include "error.h"
 #include "validation.h"
 
+#define NETPLAN_VERSION_MIN	2
+#define NETPLAN_VERSION_MAX	3
+
 /* convenience macro to put the offset of a NetplanNetDefinition field into "void* data" */
 #define access_point_offset(field) GUINT_TO_POINTER(offsetof(NetplanWifiAccessPoint, field))
 #define addr_option_offset(field) GUINT_TO_POINTER(offsetof(NetplanAddressOptions, field))
@@ -1818,12 +1822,6 @@ handle_dhcp_identifier(yaml_document_t* doc, yaml_node_t* node, const void* data
  * Grammar and handlers for tunnels
  ****************************************************/
 
-const char*
-tunnel_mode_to_string(NetplanTunnelMode mode)
-{
-    return netplan_tunnel_mode_table[mode];
-}
-
 static gboolean
 handle_tunnel_addr(yaml_document_t* doc, yaml_node_t* node, const void* data, GError** error)
 {
@@ -1855,7 +1853,7 @@ handle_tunnel_mode(yaml_document_t* doc, yaml_node_t* node, const void* _, GErro
 
     // Skip over unknown (0) tunnel mode.
     for (i = 1; i < NETPLAN_TUNNEL_MODE_MAX_; ++i) {
-        if (g_strcmp0(netplan_tunnel_mode_table[i], key) == 0) {
+        if (g_strcmp0(np_tunnel_mode_name(i), key) == 0) {
             cur_netdef->tunnel.mode = i;
             return TRUE;
         }

diff --git a/src/parse.h b/src/parse.h
@@ -64,13 +64,6 @@ typedef enum {
     NETPLAN_BACKEND_MAX_,
 } NetplanBackend;
 
-static const char* const netplan_backend_to_name[NETPLAN_BACKEND_MAX_] = {
-        [NETPLAN_BACKEND_NONE] = "none",
-        [NETPLAN_BACKEND_NETWORKD] = "networkd",
-        [NETPLAN_BACKEND_NM] = "NetworkManager",
-        [NETPLAN_BACKEND_OVS] = "OpenVSwitch",
-};
-
 typedef enum {
     NETPLAN_RA_MODE_KERNEL,
     NETPLAN_RA_MODE_ENABLED,
@@ -414,13 +407,6 @@ typedef enum {
     NETPLAN_WIFI_MODE_MAX_
 } NetplanWifiMode;
 
-static const char* const netplan_wifi_mode_to_str[NETPLAN_WIFI_MODE_MAX_] = {
-    [NETPLAN_WIFI_MODE_INFRASTRUCTURE] = "infrastructure",
-    [NETPLAN_WIFI_MODE_ADHOC] = "adhoc",
-    [NETPLAN_WIFI_MODE_AP] = "ap",
-    [NETPLAN_WIFI_MODE_OTHER] = NULL,
-};
-
 typedef struct {
     char *endpoint;
     char *public_key;
@@ -519,7 +505,6 @@ gboolean netplan_parse_yaml(const char* filename, GError** error);
 GHashTable* netplan_finish_parse(GError** error);
 guint netplan_clear_netdefs();
 NetplanBackend netplan_get_global_backend();
-const char* tunnel_mode_to_string(NetplanTunnelMode mode);
 NetplanNetDefinition* netplan_netdef_new(const char* id, NetplanDefType type, NetplanBackend renderer);
 void reset_netdef(NetplanNetDefinition *netdef, NetplanDefType type, NetplanBackend renderer);
 

diff --git a/src/util.c b/src/util.c
@@ -24,6 +24,7 @@
 
 #include "util.h"
 #include "netplan.h"
+#include "names.h"
 
 GHashTable* wifi_frequency_24;
 GHashTable* wifi_frequency_5;
@@ -229,7 +230,7 @@ netplan_delete_connection(const char* id, const char* rootdir)
 
     filename = g_path_get_basename(nd->filename);
     filename[strlen(filename) - 5] = '\0'; //stip ".yaml" suffix
-    del = g_strdup_printf("network.%s.%s=NULL", netplan_def_type_to_str[nd->type], id);
+    del = g_strdup_printf("network.%s.%s=NULL", np_def_type_name(nd->type), id);
     netplan_clear_netdefs();
 
     /* TODO: refactor logic to actually be inside the library instead of spawning another process */