[4.1] 26bb5f0 Prevent storage backends name collisions

[PÃ¥l Hermunn Johansen]

commit 26bb5f0252395ebd600948d5109214cb02359fd0
Author: Pål Hermunn Johansen <hermunn at varnish-software.com>
Date:   Thu Jun 15 12:03:39 2017 +0200

    Prevent storage backends name collisions
    
    This is a backport of 986763198 by Dridi Boukelmoune, but this patch
    is a bit different from the original.
    
    Now name collisions with transient will be reported in a sane way
    instead of producing an assert. Normal name collisions are also more
    readable.
    
    The test case is unmodified, thanks to resent back porting efforts in
    varnishtest.
    
    Fixes #2321
    
    Conflicts:
    	bin/varnishd/storage/mgt_stevedore.c

diff --git a/bin/varnishd/storage/mgt_stevedore.c b/bin/varnishd/storage/mgt_stevedore.c
index 109290f..247574c 100644
--- a/bin/varnishd/storage/mgt_stevedore.c
+++ b/bin/varnishd/storage/mgt_stevedore.c
@@ -116,6 +116,28 @@ static const struct choice STV_choice[] = {
 	{ NULL,		NULL }
 };
 
+static void
+stv_check_ident(const char *spec, const char *ident)
+{
+	struct stevedore *stv;
+	unsigned found = 0;
+
+	if (!strcmp(ident, TRANSIENT_STORAGE))
+		found = (stv_transient != NULL);
+	else {
+		VTAILQ_FOREACH(stv, &stv_stevedores, list) {
+			CHECK_OBJ(stv, STEVEDORE_MAGIC);
+			if (!strcmp(stv->ident, ident)) {
+				found = 1;
+				break;
+			}
+		}
+	}
+
+	if (found)
+		ARGV_ERR("(-s %s) '%s' is already defined\n", spec, ident);
+}
+
 void
 STV_Config(const char *spec)
 {
@@ -170,17 +192,12 @@ STV_Config(const char *spec)
 		bprintf(stv->ident, "%.*s", l, spec);
 	}
 
-	VTAILQ_FOREACH(stv2, &stv_stevedores, list) {
-		if (strcmp(stv2->ident, stv->ident))
-			continue;
-		ARGV_ERR("(-s%s=%s) already defined once\n",
-		    stv->ident, stv->name);
-	}
+	stv_check_ident(spec, stv->ident);
 
 	if (stv->init != NULL)
 		stv->init(stv, ac, av);
 	else if (ac != 0)
-		ARGV_ERR("(-s%s) too many arguments\n", stv->name);
+		ARGV_ERR("(-s %s) too many arguments\n", stv->name);
 
 	AN(stv->alloc);
 	if (stv->allocobj == NULL)
diff --git a/bin/varnishtest/tests/r02321.vtc b/bin/varnishtest/tests/r02321.vtc
new file mode 100644
index 0000000..33c7658
--- /dev/null
+++ b/bin/varnishtest/tests/r02321.vtc
@@ -0,0 +1,19 @@
+varnishtest "Storage name collisions"
+
+# intentional collision
+shell -err -expect "Error: (-s main=malloc,100m) 'main' is already defined" {
+	exec varnishd -a :0 -f '' -n ${tmpdir} \
+		-s main=malloc,10m -s main=malloc,100m
+}
+
+# pseudo-accidental collision
+shell -err -expect "Error: (-s s0=malloc,100m) 's0' is already defined" {
+	exec varnishd -a :0 -f '' -n ${tmpdir} \
+		-s malloc,10m -s s0=malloc,100m
+}
+
+# Transient collision
+shell -err -expect "Error: (-s Transient=malloc,100m) 'Transient' is already defined" {
+	exec varnishd -a :0 -f '' -n ${tmpdir} \
+		-s Transient=malloc,10m -s Transient=malloc,100m
+}