Re: [PATCH 08/16] xenParseVif: Refactor parser

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On a Tuesday in 2021, Peter Krempa wrote:
Use g_strsplit to split the string and avoid use of stack'd strings.

Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
---
src/libxl/xen_common.c | 136 ++++++++++++++---------------------------
1 file changed, 46 insertions(+), 90 deletions(-)

diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c
index 781483f496..09c74dcb53 100644
--- a/src/libxl/xen_common.c
+++ b/src/libxl/xen_common.c
@@ -1141,104 +1141,61 @@ xenParseVif(char *entry, const char *vif_typename)
{
    virDomainNetDefPtr net = NULL;
    virDomainNetDefPtr ret = NULL;
-    char *script = NULL;
-    char model[10];
-    char type[10];
-    char ip[128];
-    char mac[18];
-    char bridge[50];
-    char vifname[50];
-    char rate[50];
-    char *key;
-
-    bridge[0] = '\0';
-    mac[0] = '\0';
-    ip[0] = '\0';
-    model[0] = '\0';
-    type[0] = '\0';
-    vifname[0] = '\0';
-    rate[0] = '\0';
-
-    key = entry;
-    while (key) {
-        char *data;
-        char *nextkey = strchr(key, ',');
-
-        if (!(data = strchr(key, '=')))
+    g_auto(GStrv) keyvals = NULL;
+    GStrv keyval;
+    g_autofree char *script = NULL;
+    g_autofree char *model = NULL;
+    g_autofree char *type = NULL;
+    g_autofree char *ip = NULL;
+    g_autofree char *mac = NULL;
+    g_autofree char *bridge = NULL;
+    g_autofree char *vifname = NULL;
+    g_autofree char *rate = NULL;
+

These can be marked as const instead of autofree. Their values are
processed before 'keyvals' gets freed at the end of the function.

It would result in less lines and no g_free/g_autofree mixing.

Jano

e    keyvals = g_strsplit(entry, ",", 0);
+
+    for (keyval = keyvals; keyval && *keyval; keyval++) {
+        const char *key = *keyval;
+        char *val = strchr(key, '=');
+
+        virSkipSpaces(&key);
+
+        if (!val)
            return NULL;
-        data++;
+
+        val++;

        if (STRPREFIX(key, "mac=")) {
-            int len = nextkey ? (nextkey - data) : strlen(data);
-            if (virStrncpy(mac, data, len, sizeof(mac)) < 0) {
-                virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("MAC address %s too big for destination"),
-                               data);
-                return NULL;
-            }
+            g_clear_pointer(&mac, g_free);
+            mac = g_strdup(val);
        } else if (STRPREFIX(key, "bridge=")) {
-            int len = nextkey ? (nextkey - data) : strlen(data);
-            if (virStrncpy(bridge, data, len, sizeof(bridge)) < 0) {
-                virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("Bridge %s too big for destination"),
-                               data);
-                return NULL;
-            }
+            g_clear_pointer(&bridge, g_free);
+            bridge = g_strdup(val);
        } else if (STRPREFIX(key, "script=")) {
-            int len = nextkey ? (nextkey - data) : strlen(data);
-            VIR_FREE(script);
-            script = g_strndup(data, len);
+            g_clear_pointer(&script, g_free);
+            script = g_strdup(val);
        } else if (STRPREFIX(key, "model=")) {
-            int len = nextkey ? (nextkey - data) : strlen(data);
-            if (virStrncpy(model, data, len, sizeof(model)) < 0) {
-                virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("Model %s too big for destination"),
-                               data);
-                return NULL;
-            }
+            g_clear_pointer(&model, g_free);
+            model = g_strdup(val);
        } else if (STRPREFIX(key, "type=")) {
-            int len = nextkey ? (nextkey - data) : strlen(data);
-            if (virStrncpy(type, data, len, sizeof(type)) < 0) {
-                virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("Type %s too big for destination"),
-                               data);
-                return NULL;
-            }
+            g_clear_pointer(&type, g_free);
+            type = g_strdup(val);
        } else if (STRPREFIX(key, "vifname=")) {
-            int len = nextkey ? (nextkey - data) : strlen(data);
-            if (virStrncpy(vifname, data, len, sizeof(vifname)) < 0) {
-                virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("Vifname %s too big for destination"),
-                               data);
-                return NULL;
-            }
+            g_clear_pointer(&vifname, g_free);
+            vifname = g_strdup(val);
        } else if (STRPREFIX(key, "ip=")) {
-            int len = nextkey ? (nextkey - data) : strlen(data);
-            if (virStrncpy(ip, data, len, sizeof(ip)) < 0) {
-                virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("IP %s too big for destination"), data);
-                return NULL;
-            }
+            g_clear_pointer(&ip, g_free);
+            ip = g_strdup(val);
        } else if (STRPREFIX(key, "rate=")) {
-            int len = nextkey ? (nextkey - data) : strlen(data);
-            if (virStrncpy(rate, data, len, sizeof(rate)) < 0) {
-                virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("rate %s too big for destination"), data);
-                return NULL;
-            }
+            g_clear_pointer(&rate, g_free);
+            rate = g_strdup(val);
        }
-
-        while (nextkey && (nextkey[0] == ',' ||
-                           nextkey[0] == ' ' ||
-                           nextkey[0] == '\t'))
-            nextkey++;
-        key = nextkey;
    }

    if (!(net = virDomainNetDefNew(NULL)))
        goto cleanup;

-    if (mac[0]) {
+    if (mac) {
        if (virMacAddrParse(mac, &net->mac) < 0) {
            virReportError(VIR_ERR_INTERNAL_ERROR,
                           _("malformed mac address '%s'"), mac);
@@ -1246,18 +1203,18 @@ xenParseVif(char *entry, const char *vif_typename)
        }
    }

-    if (bridge[0] || STREQ_NULLABLE(script, "vif-bridge") ||
+    if (bridge || STREQ_NULLABLE(script, "vif-bridge") ||
        STREQ_NULLABLE(script, "vif-vnic")) {
        net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
    } else {
        net->type = VIR_DOMAIN_NET_TYPE_ETHERNET;
    }

-    if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE && bridge[0]) {
+    if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE && bridge) {
        if (xenParseVifBridge(net, bridge) < 0)
            goto cleanup;
    }
-    if (ip[0]) {
+    if (ip) {
        char **ip_list = g_strsplit(ip, " ", 0);
        size_t i;

@@ -1276,18 +1233,18 @@ xenParseVif(char *entry, const char *vif_typename)
    if (script && script[0])
        net->script = g_strdup(script);

-    if (model[0]) {
+    if (model) {
        if (virDomainNetSetModelString(net, model) < 0)
            goto cleanup;
    } else {
-        if (type[0] && STREQ(type, vif_typename))
+        if (type && STREQ(type, vif_typename))
            net->model = VIR_DOMAIN_NET_MODEL_NETFRONT;
    }

-    if (vifname[0])
+    if (vifname && vifname[0])
        net->ifname = g_strdup(vifname);

-    if (rate[0]) {
+    if (rate) {
        virNetDevBandwidthPtr bandwidth;
        unsigned long long kbytes_per_sec;

@@ -1304,7 +1261,6 @@ xenParseVif(char *entry, const char *vif_typename)

 cleanup:
    virDomainNetDefFree(net);
-    VIR_FREE(script);
    return ret;
}

--
2.29.2

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux