change the approach for strnvis

instead of making things more obscure via gmid_strnvis(), let's just check for strnvis with -Werror so we can swap the OS broken implementation with the bundled OpenBSD one.
omar-polo · Jun 8, 2024 · 4010301 · 4010301
1 parent fcf3f1f
commit 4010301
Show file tree

Hide file tree

Showing 8 changed files with 8 additions and 48 deletions.
diff --git a/configure b/configure
@@ -275,7 +275,6 @@ fi
 HAVE_ENDIAN_H=0
 HAVE_SYS_ENDIAN_H=0
 HAVE_MACHINE_ENDIAN=0
-HAVE_BROKEN_STRNVIS=0
 
 runtest endian_h	ENDIAN_H	|| \
 runtest sys_endian_h	SYS_ENDIAN_H	|| \
@@ -316,12 +315,12 @@ runtest strtonum	STRTONUM	-D_OPENBSD_SOURCE	|| true
 runtest timingsafe_memcmp TIMINGSAFE_MEMCMP || true
 runtest tree_h		TREE_H		|| true
 runtest vasprintf	VASPRINTF	-D_GNU_SOURCE || true
-runtest vis		VIS		-DLIBBSD_OPENBSD_VIS	|| true
 
-if [ ${HAVE_VIS} -eq 1 ]; then
-	if singletest broken-strnvis BROKEN_STRNVIS "-Wall -Werror"; then
-		HAVE_BROKEN_STRNVIS=1
-	fi
+# strnvis is a bit special since NetBSD, FreeBSD and MacOS have
+# the broken version with the wrong semantics and arguments.
+# Hence the -Wall -Werror check.
+if ! singletest strnvis STRNVIS "-Wall -Werror"; then
+	CFLAGS="-I ${PWD}/compat/vis ${CFLAGS} ${CFLAGS}"
 fi
 
 if [ ${HAVE_ARC4RANDOM} -eq 1 -a ${HAVE_ARC4RANDOM_BUF} -eq 0 ]; then
@@ -395,10 +394,6 @@ if [ ${HAVE_QUEUE_H} -eq 0 -o ${HAVE_IMSG} -eq 0 -o ${HAVE_TREE_H} -eq 0 ]; then
 	CFLAGS="${CFLAGS} -I ${PWD}/compat"
 fi
 
-if [ ${HAVE_VIS} -eq 0 ]; then
-	CFLAGS="${CFLAGS} -I ${PWD}/compat/vis"
-fi
-
 if [ $HAVE_LIBEVENT2 -eq 1 ]; then
 	CFLAGS="$CFLAGS -DHAVE_LIBEVENT2=1"
 fi
@@ -655,8 +650,6 @@ cat <<__HEREDOC__
 #  define HOST_NAME_MAX 255
 # endif
 #endif
-
-#define HAVE_BROKEN_STRNVIS ${HAVE_BROKEN_STRNVIS}
 __HEREDOC__
 
 echo "file config.h: written" 1>&2

diff --git a/ge.c b/ge.c
@@ -83,7 +83,7 @@ log_request(struct client *c, int code, const char *meta)
 			strlcpy(cntmp, subj + 4, sizeof(cntmp));
 			if ((n = strchr(cntmp, '/')) != NULL)
 				*n = '\0';
-			gmid_strnvis(cn, cntmp, sizeof(cn), VIS_WHITE|VIS_DQ);
+			strnvis(cn, cntmp, sizeof(cn), VIS_WHITE|VIS_DQ);
 		}
 	}
 

diff --git a/gmid.c b/gmid.c
@@ -140,7 +140,7 @@ log_request(struct client *c, int code, const char *meta)
 			strlcpy(cntmp, subj + 4, sizeof(cntmp));
 			if ((n = strchr(cntmp, '/')) != NULL)
 				*n = '\0';
-			gmid_strnvis(cn, cntmp, sizeof(cn), VIS_WHITE|VIS_DQ);
+			strnvis(cn, cntmp, sizeof(cn), VIS_WHITE|VIS_DQ);
 		}
 	}
 

diff --git a/gmid.h b/gmid.h
@@ -469,6 +469,5 @@ EVP_PKEY	*ssl_load_pkey(const uint8_t *, size_t);
 struct vhost	*new_vhost(void);
 struct location	*new_location(void);
 struct proxy	*new_proxy(void);
-int		 gmid_strnvis(char *, const char *, size_t, int);
 
 #endif
diff --git a/have/Makefile b/have/Makefile
@@ -7,7 +7,6 @@ DISTFILES =	ASN1_time_parse.c \
 		X509_LOOKUP_mem.c \
 		arc4random.c \
 		arc4random_buf.c \
-		broken-strnvis.c \
 		endian_h.c \
 		err.c \
 		explicit_bzero.c \
@@ -35,12 +34,12 @@ DISTFILES =	ASN1_time_parse.c \
 		setresuid.c \
 		strlcat.c \
 		strlcpy.c \
+		strnvis.c \
 		strtonum.c \
 		sys_endian_h.c \
 		timingsafe_memcmp.c \
 		tree_h.c \
 		vasprintf.c \
-		vis.c \
 		wait_any.c
 
 all:

diff --git a/have/broken-strnvis.c b/have/broken-strnvis.c
diff --git a/have/vis.c → have/strnvis.c b/have/vis.c → have/strnvis.c
diff --git a/utils.c b/utils.c
@@ -497,19 +497,3 @@ new_proxy(void)
 	p->protocols = TLS_PROTOCOLS_DEFAULT;
 	return p;
 }
-
-/*
- * I can't rant enough about this situation.  As far as I've understood,
- * OpenBSD introduced strnvis(3) with a signature, then NetBSD did it but
- * with a incompatible signature.  FreeBSD followed NetBSD.  libbsd followed
- * OpenBSD but now is thinking to switch.  WTF?
- */
-int
-gmid_strnvis(char *dst, const char *src, size_t destsize, int flag)
-{
-#if HAVE_BROKEN_STRNVIS
-	return strnvis(dst, destsize, src, flag);
-#else
-	return strnvis(dst, src, destsize, flag);
-#endif
-}