[master] 45cdc60e9 mgt: Simplify key=val argument handling

Nils Goroll nils.goroll at uplex.de
Thu Feb 13 16:15:06 UTC 2025


commit 45cdc60e96e9cc2052c523239e99e774baf656f1
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Wed Feb 12 20:39:12 2025 +0100

    mgt: Simplify key=val argument handling
    
    we add a simple utility function to return the pointer to the thing after "key="
    if the first argument begins with "key=".
    
    Note that this is no worse than a macro with a "reasonable good" compiler *) and
    default (-O2) optimization, because the function call gets inlined and the
    strlen() turned into a constant.
    
      0x00000000000a1bd1 <+49>:     lea    0x6ed49(%rip),%r14        # 0x110921
      ...
      0x00000000000a1c01 <+97>:    mov    $0x5,%edx
      0x00000000000a1c06 <+102>:   mov    %rbx,%rdi
      0x00000000000a1c09 <+105>:   mov    %r14,%rsi
      0x00000000000a1c0c <+108>:   call   0x2d360 <strncmp at plt>
      0x00000000000a1c11 <+113>:   lea    0x5(%rbx),%rbp
      0x00000000000a1c15 <+117>:   test   %eax,%eax
    
      (gdb) p (const char *)0x110921
      $2 = 0x110921 "user="
    
    Replaces #4202
    Polishes 4001ea251bac6924c2e86bac3d0f2c8d93c96bd8
    
    *) tested:
       gcc version 12.2.0 (Debian 12.2.0-14)
       Debian clang version 14.0.6

diff --git a/bin/varnishd/acceptor/mgt_acceptor_uds.c b/bin/varnishd/acceptor/mgt_acceptor_uds.c
index c10fa7af6..51c558129 100644
--- a/bin/varnishd/acceptor/mgt_acceptor_uds.c
+++ b/bin/varnishd/acceptor/mgt_acceptor_uds.c
@@ -179,10 +179,9 @@ vca_uds_open(char **av, struct listen_arg *la, const char **err)
 	heritage.min_vcl_version = vmax(heritage.min_vcl_version, 41U);
 
 	for (int i = 0; av[i] != NULL; i++) {
-		char *eq, *val;
-		int len;
+		const char *val;
 
-		if ((eq = strchr(av[i], '=')) == NULL) {
+		if (strchr(av[i], '=') == NULL) {
 			if (xp != NULL)
 				ARGV_ERR("Too many protocol sub-args"
 				    " in -a (%s)\n", av[i]);
@@ -195,41 +194,35 @@ vca_uds_open(char **av, struct listen_arg *la, const char **err)
 			ARGV_ERR("Invalid sub-arg %s"
 			    " in -a\n", av[i]);
 
-		val = eq + 1;
-		len = eq - av[i];
-		assert(len >= 0);
-		if (len == 0)
-			ARGV_ERR("Invalid sub-arg %s in -a\n", av[i]);
-
-		if (strncmp(av[i], "user", len) == 0) {
-			if (pwd != NULL)
-				ARGV_ERR("Too many user sub-args in -a (%s)\n",
-					 av[i]);
+		val = keyval(av[i], "user=");
+		if (val != NULL && pwd != NULL)
+			ARGV_ERR("Too many user sub-args in -a (%s)\n", av[i]);
+		if (val != NULL) {
 			pwd = getpwnam(val);
 			if (pwd == NULL)
 				ARGV_ERR("Unknown user %s in -a\n", val);
 			continue;
 		}
 
-		if (strncmp(av[i], "group", len) == 0) {
-			if (grp != NULL)
-				ARGV_ERR("Too many group sub-args in -a (%s)\n",
-					 av[i]);
+		val = keyval(av[i], "group=");
+		if (val != NULL && grp != NULL)
+			ARGV_ERR("Too many group sub-args in -a (%s)\n", av[i]);
+		if (val != NULL) {
 			grp = getgrnam(val);
 			if (grp == NULL)
 				ARGV_ERR("Unknown group %s in -a\n", val);
 			continue;
 		}
 
-		if (strncmp(av[i], "mode", len) == 0) {
+		val = keyval(av[i], "mode=");
+		if (val != NULL && mode != 0)
+			ARGV_ERR("Too many mode sub-args in -a (%s)\n", av[i]);
+		if (val != NULL && *val == '\0')
+			ARGV_ERR("Empty mode sub-arg in -a\n");
+		if (val != NULL) {
 			long m;
 			char *p;
 
-			if (mode != 0)
-				ARGV_ERR("Too many mode sub-args in -a (%s)\n",
-					 av[i]);
-			if (*val == '\0')
-				ARGV_ERR("Empty mode sub-arg in -a\n");
 			errno = 0;
 			m = strtol(val, &p, 8);
 			if (*p != '\0')
diff --git a/bin/varnishd/mgt/mgt.h b/bin/varnishd/mgt/mgt.h
index f98862363..172288ab2 100644
--- a/bin/varnishd/mgt/mgt.h
+++ b/bin/varnishd/mgt/mgt.h
@@ -38,6 +38,7 @@
 
 #include <stdint.h>
 
+#include "string.h"
 #include <sys/types.h>
 
 #include "vdef.h"
@@ -64,6 +65,16 @@ extern struct vev_root	*mgt_evb;
 extern unsigned		d_flag;
 extern int		exit_status;
 
+/* option=value argument parse helper */
+static inline const char *
+keyval(const char *p, const char *name)
+{
+	size_t l = strlen(name);
+	if (strncmp(p, name, l))
+		return (NULL);
+	return (p + l);
+}
+
 /* builtin_vcl.c */
 
 extern const char * const builtin_vcl;
diff --git a/bin/varnishd/mgt/mgt_jail_solaris.c b/bin/varnishd/mgt/mgt_jail_solaris.c
index d35d93726..de685f8f8 100644
--- a/bin/varnishd/mgt/mgt_jail_solaris.c
+++ b/bin/varnishd/mgt/mgt_jail_solaris.c
@@ -323,13 +323,14 @@ static int v_matchproto_(jail_init_f)
 vjs_init(char **args)
 {
 	priv_set_t **sets, *permitted, *inheritable, *user = NULL;
-	const char *e;
+	const char *e, *val;
 	int vj, vs;
 
 	if (args != NULL && *args != NULL) {
 		for (;*args != NULL; args++) {
-			if (!strncmp(*args, "worker=", 7)) {
-				user = priv_str_to_set((*args) + 7, ",", &e);
+			val = keyval(*args, "worker=");
+			if (val != NULL) {
+				user = priv_str_to_set(val, ",", &e);
 				if (user == NULL)
 					ARGV_ERR(
 					    "-jsolaris: parsing worker= "
diff --git a/bin/varnishd/mgt/mgt_jail_unix.c b/bin/varnishd/mgt/mgt_jail_unix.c
index 66e088253..9ca2f234b 100644
--- a/bin/varnishd/mgt/mgt_jail_unix.c
+++ b/bin/varnishd/mgt/mgt_jail_unix.c
@@ -117,9 +117,15 @@ vju_getccgid(const char *arg)
 /**********************************************************************
  */
 
+// avoid line breaks
+#define JERR(what, val)						\
+	ARGV_ERR("Unix jail: %s " what " not found.\n", val)
+
 static int v_matchproto_(jail_init_f)
 vju_init(char **args)
 {
+	const char *val;
+
 	if (args == NULL) {
 		/* Autoconfig */
 		if (geteuid() != 0)
@@ -132,31 +138,22 @@ vju_init(char **args)
 			ARGV_ERR("Unix Jail: Must be root.\n");
 
 		for (;*args != NULL; args++) {
-			const char * const a_user = "user=";
-			const size_t l_user = strlen(a_user);
-			if (!strncmp(*args, a_user, l_user)) {
-				if (vju_getuid((*args) + l_user))
-					ARGV_ERR(
-					    "Unix jail: %s user not found.\n",
-					    (*args) + 5);
+			val = keyval(*args, "user=");
+			if (val != NULL) {
+				if (vju_getuid(val))
+					JERR("user", val);
 				continue;
 			}
-			const char * const a_workuser = "workuser=";
-			const size_t l_workuser = strlen(a_workuser);
-			if (!strncmp(*args, a_workuser, l_workuser)) {
-				if (vju_getwrkuid((*args) + l_workuser))
-					ARGV_ERR(
-					    "Unix jail: %s user not found.\n",
-					    (*args) + 9);
+			val = keyval(*args, "workuser=");
+			if (val != NULL) {
+				if (vju_getwrkuid(val))
+					JERR("user", val);
 				continue;
 			}
-			const char * const a_ccgroup = "ccgroup=";
-			const size_t l_ccgroup = strlen(a_ccgroup);
-			if (!strncmp(*args, "ccgroup=", l_ccgroup)) {
-				if (vju_getccgid((*args) + l_ccgroup))
-					ARGV_ERR(
-					    "Unix jail: %s group not found.\n",
-					    (*args) + 8);
+			val = keyval(*args, "ccgroup=");
+			if (val != NULL) {
+				if (vju_getccgid(val))
+					JERR("group", val);
 				continue;
 			}
 			ARGV_ERR("Unix jail: unknown sub-argument '%s'\n",
@@ -164,8 +161,7 @@ vju_init(char **args)
 		}
 
 		if (vju_user == NULL && vju_getuid(VARNISH_USER))
-			ARGV_ERR("Unix jail: %s user not found.\n",
-			    VARNISH_USER);
+			JERR("user", VARNISH_USER);
 	}
 
 	AN(vju_user);
@@ -187,6 +183,8 @@ vju_init(char **args)
 	return (0);
 }
 
+#undef JERR
+
 static void v_matchproto_(jail_master_f)
 vju_master(enum jail_master_e jme)
 {


More information about the varnish-commit mailing list