[pve-devel] [PATCH 1/3] pmxcfs: Use g_string_append when appropriate

Maximiliano Sandoval m.sandoval at proxmox.com
Thu Feb 20 13:48:02 CET 2025


g_string_append_printf should only be used when there is something to
format.

Signed-off-by: Maximiliano Sandoval <m.sandoval at proxmox.com>
---

Some micro optimizations for GStrings usage.

The following script can be used to verify the claim that the replacements are drop-in:

```c
/* test.c */
#include <glib.h>

int
main () {
    GString *a = g_string_new ("Hello");
    GString *b = g_string_new ("Hello");

    g_string_append (a, "\n");
    g_string_append_c (b, '\n');
    g_assert_cmpstr (a->str, ==, b->str);

    g_string_printf (a, "{\n");
    g_string_assign (b, "{\n");
    g_assert_cmpstr (a->str, ==, b->str);

    g_string_append_printf (a, "\"data\": [\n");
    g_string_append (b, "\"data\": [\n");
    g_assert_cmpstr (a->str, ==, b->str);
}
```

which can be compiled with
```
cc `pkg-config --cflags glib-2.0` test.c -o test `pkg-config --libs glib-2.0`
```

assuming `libglib2.0-0` is installed.


 src/pmxcfs/logger.c | 12 ++++++------
 src/pmxcfs/status.c | 44 ++++++++++++++++++++++----------------------
 2 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/src/pmxcfs/logger.c b/src/pmxcfs/logger.c
index 3792650..0be4be6 100644
--- a/src/pmxcfs/logger.c
+++ b/src/pmxcfs/logger.c
@@ -163,9 +163,9 @@ clog_dump_json(
 
 	uint32_t cpos = clog->cpos;
 
-	g_string_append_printf(str, "{\n");
+	g_string_append(str, "{\n");
 
-	g_string_append_printf(str, "\"data\": [\n");
+	g_string_append(str, "\"data\": [\n");
 
 	guint count = 0;
 	while (cpos && (cpos <= clog->cpos || cpos > (clog->cpos + CLOG_MAX_ENTRY_SIZE))) {
@@ -189,7 +189,7 @@ clog_dump_json(
 		char *msg = tag + cur->tag_len;
 
 		if (count)
-			g_string_append_printf(str, ",\n");
+			g_string_append(str, ",\n");
 
 		g_string_append_printf(str, "{\"uid\": %u, \"time\": %u, \"pri\": %d, \"tag\": \"%s\", "
 				       "\"pid\": %u, \"node\": \"%s\", \"user\": \"%s\", "
@@ -201,10 +201,10 @@ clog_dump_json(
 	}
 
 	if (count)
-		g_string_append_printf(str, "\n");
+		g_string_append(str, "\n");
 
-	g_string_append_printf(str, "]\n");
-	g_string_append_printf(str, "}\n");
+	g_string_append(str, "]\n");
+	g_string_append(str, "}\n");
 
 }
 
diff --git a/src/pmxcfs/status.c b/src/pmxcfs/status.c
index ff5fcc4..e76c150 100644
--- a/src/pmxcfs/status.c
+++ b/src/pmxcfs/status.c
@@ -321,7 +321,7 @@ cfs_create_memberlist_msg(
 
 	g_mutex_lock (&mutex);
 
-	g_string_append_printf(str,"{\n");
+	g_string_append(str, "{\n");
 
 	guint nodecount = 0;
 
@@ -334,14 +334,14 @@ cfs_create_memberlist_msg(
 		g_string_append_printf(str, "\"nodename\": \"%s\",\n", cfs.nodename);
 		g_string_append_printf(str, "\"version\": %u,\n", cfs_status.clinfo_version);
 
-		g_string_append_printf(str, "\"cluster\": { ");
+		g_string_append(str, "\"cluster\": { ");
 		g_string_append_printf(str, "\"name\": \"%s\", \"version\": %d, "
 				       "\"nodes\": %d, \"quorate\": %d ",
 				       clinfo->cluster_name, clinfo->cman_version,
 				       nodecount, cfs_status.quorate);
 
-		g_string_append_printf(str,"},\n");
-		g_string_append_printf(str,"\"nodelist\": {\n");
+		g_string_append(str, "},\n");
+		g_string_append(str, "\"nodelist\": {\n");
 
 		GHashTable *ht = clinfo->nodes_byid;
 		GHashTableIter iter;
@@ -352,7 +352,7 @@ cfs_create_memberlist_msg(
 		int i = 0;
 		while (g_hash_table_iter_next (&iter, &key, &value)) {
 			cfs_clnode_t *node = (cfs_clnode_t *)value;
-			if (i) g_string_append_printf(str, ",\n");
+			if (i) g_string_append(str, ",\n");
 			i++;
 
 			g_string_append_printf(str, "  \"%s\": { \"id\": %d, \"online\": %d",
@@ -364,16 +364,16 @@ cfs_create_memberlist_msg(
 				g_string_append_printf(str, ", \"ip\": \"%s\"", ip);
 			}
 
-			g_string_append_printf(str, "}");
+			g_string_append(str, "}");
 
 		}
-		g_string_append_printf(str,"\n  }\n");
+		g_string_append(str, "\n  }\n");
 	} else {
 		g_string_append_printf(str, "\"nodename\": \"%s\",\n", cfs.nodename);
 		g_string_append_printf(str, "\"version\": %u\n", cfs_status.clinfo_version);
 	}
 
-	g_string_append_printf(str,"}\n");
+	g_string_append(str, "}\n");
 
 	g_mutex_unlock (&mutex);
 
@@ -570,12 +570,12 @@ dump_kvstore_versions(
 	int i = 0;
 	while (g_hash_table_iter_next (&iter, &key, &value)) {
 		kventry_t *entry = (kventry_t *)value;
-		if (i) g_string_append_printf(str, ",\n");
+		if (i) g_string_append(str, ",\n");
 		i++;
 		g_string_append_printf(str,"\"%s\": %u", entry->key, entry->version);
 	}
 
-	g_string_append_printf(str, "}\n");
+	g_string_append(str, "}\n");
 }
 
 int
@@ -585,7 +585,7 @@ cfs_create_version_msg(GString *str)
 
 	g_mutex_lock (&mutex);
 
-	g_string_append_printf(str,"{\n");
+	g_string_append(str, "{\n");
 
 	g_string_append_printf(str, "\"starttime\": %lu,\n", (unsigned long)cfs_status.start_time);
 
@@ -599,7 +599,7 @@ cfs_create_version_msg(GString *str)
 				       memdb_change_array[i].version);
 	}
 
-	g_string_append_printf(str, "\"kvstore\": {\n");
+	g_string_append(str, "\"kvstore\": {\n");
 
 	dump_kvstore_versions(str, cfs_status.kvhash, cfs.nodename);
 
@@ -616,14 +616,14 @@ cfs_create_version_msg(GString *str)
 			cfs_clnode_t *node = (cfs_clnode_t *)value;
 			if (!node->kvhash)
 				continue;
-			g_string_append_printf(str, ",\n");
+			g_string_append(str, ",\n");
 			dump_kvstore_versions(str, node->kvhash, node->name);
 		}
 	}
 
-	g_string_append_printf(str,"}\n");
+	g_string_append(str, "}\n");
 
-	g_string_append_printf(str,"}\n");
+	g_string_append(str, "}\n");
 
 	g_mutex_unlock (&mutex);
 
@@ -774,7 +774,7 @@ cfs_create_vmlist_msg(GString *str)
 
 	g_mutex_lock (&mutex);
 
-	g_string_append_printf(str,"{\n");
+	g_string_append(str, "{\n");
 
 	GHashTable *ht = cfs_status.vmlist;
 
@@ -785,7 +785,7 @@ cfs_create_vmlist_msg(GString *str)
 	} else {
 		g_string_append_printf(str,"\"version\": %u,\n", cfs_status.vmlist_version);
 
-		g_string_append_printf(str,"\"ids\": {\n");
+		g_string_append(str, "\"ids\": {\n");
 
 		GHashTableIter iter;
 		gpointer key, value;
@@ -798,16 +798,16 @@ cfs_create_vmlist_msg(GString *str)
 			const char *type = vminfo_type_to_string(vminfo);
 
 			if (!first)
-				g_string_append_printf(str, ",\n");
+				g_string_append(str, ",\n");
 			first = 0;
 
 			g_string_append_printf(str,"\"%u\": { \"node\": \"%s\", \"type\": \"%s\", \"version\": %u }",
 					       vminfo->vmid, vminfo->nodename, type, vminfo->version);
 		}
 
-		g_string_append_printf(str,"}\n");
+		g_string_append(str, "}\n");
 	}
-	g_string_append_printf(str,"\n}\n");
+	g_string_append(str, "\n}\n");
 
 	g_mutex_unlock (&mutex);
 
@@ -945,7 +945,7 @@ _print_found_properties(
 			g_string_append_c(str, ',');
 		} else {
 			if (!first) {
-				g_string_append_printf(str, ",\n");
+				g_string_append(str, ",\n");
 			} else {
 				first = 0;
 			}
@@ -1056,7 +1056,7 @@ ret:
 	if (path != NULL) {
 		g_string_free(path, TRUE);
 	}
-	g_string_append_printf(str,"\n}\n");
+	g_string_append(str, "\n}\n");
 	g_mutex_unlock (&mutex);
 	g_mutex_unlock (&memdb->mutex);
 	return res;
-- 
2.39.5





More information about the pve-devel mailing list