[PATCH] Differentiate between CLIS_PARAM(106) and CLIS_INTERROR(503) for parse/compile errors and critical errors.

Martin Blix Grydeland martin at varnish-software.com
Thu Apr 12 18:11:18 CEST 2012


This to enable regression testing on e.g. VCC compiler segfaults, as
these previously would return CLIS_PARAM(106) (the same as any bad
vcl), thus not really tripping the test case.
---
 bin/varnishd/mgt/mgt_vcc.c |   61 ++++++++++++++++++++++++++------------------
 include/vcli.h             |    3 +-
 include/vsub.h             |    2 +-
 lib/libvarnish/vsub.c      |   15 ++++++++---
 4 files changed, 50 insertions(+), 31 deletions(-)

diff --git a/bin/varnishd/mgt/mgt_vcc.c b/bin/varnishd/mgt/mgt_vcc.c
index 12433ad..a8981b3 100644
--- a/bin/varnishd/mgt/mgt_vcc.c
+++ b/bin/varnishd/mgt/mgt_vcc.c
@@ -215,25 +215,31 @@ run_dlopen(void *priv)
 }
 
 /*--------------------------------------------------------------------
- * Compile a VCL program, return shared object, errors in sb.
+ * Compile a VCL program, shared object in pof, errors in sb.
+ * Returns:
+ *   -2: Critical error
+ *   -1: Parse/compile error
+ *    0: Success (file name in 'pof')
  */
 
-static char *
-mgt_run_cc(const char *vcl, struct vsb *sb, int C_flag)
+static int
+mgt_run_cc(const char *vcl, struct vsb *sb, int C_flag, char **pof)
 {
 	char *csrc;
 	struct vsb *cmdsb;
 	char sf[] = "./vcl.########.c";
 	char of[sizeof sf + 1];
-	char *retval;
 	int sfd, i;
 	struct vcc_priv vp;
 
+	AN(pof);
+	*pof = NULL;
+
 	/* Create temporary C source file */
 	sfd = VFIL_tmpfile(sf);
 	if (sfd < 0) {
 		VSB_printf(sb, "Failed to create %s: %s", sf, strerror(errno));
-		return (NULL);
+		return (-2);
 	}
 	AZ(close(sfd));
 
@@ -242,9 +248,9 @@ mgt_run_cc(const char *vcl, struct vsb *sb, int C_flag)
 	vp.magic = VCC_PRIV_MAGIC;
 	vp.sf = sf;
 	vp.vcl = vcl;
-	if (VSUB_run(sb, run_vcc, &vp, "VCC-compiler", -1)) {
+	if ((i = VSUB_run(sb, run_vcc, &vp, "VCC-compiler", -1))) {
 		(void)unlink(sf);
-		return (NULL);
+		return (i);
 	}
 
 	if (C_flag) {
@@ -276,33 +282,35 @@ mgt_run_cc(const char *vcl, struct vsb *sb, int C_flag)
 	/* Ensure the file is readable to the unprivileged user */
 	if (!i) {
 		i = chmod(of, 0755);
-		if (i)
+		if (i) {
 			VSB_printf(sb, "Failed to set permissions on %s: %s",
 				   of, strerror(errno));
+			i = -2;
+		}
 	}
 
 	if (i) {
 		(void)unlink(of);
-		return (NULL);
+		return (i);
 	}
 
-	retval = strdup(of);
-	XXXAN(retval);
-	return (retval);
+	*pof = strdup(of);
+	XXXAN(*pof);
+	return (i);
 }
 
 /*--------------------------------------------------------------------*/
 
-static char *
-mgt_VccCompile(struct vsb **sb, const char *b, int C_flag)
+static int
+mgt_VccCompile(struct vsb **sb, const char *b, int C_flag, char **pvf)
 {
-	char *vf;
+	int i;
 
 	*sb = VSB_new_auto();
 	XXXAN(*sb);
-	vf = mgt_run_cc(b, *sb, C_flag);
+	i = mgt_run_cc(b, *sb, C_flag, pvf);
 	AZ(VSB_finish(*sb));
-	return (vf);
+	return (i);
 }
 
 /*--------------------------------------------------------------------*/
@@ -358,7 +366,6 @@ mgt_vcc_delbyname(const char *name)
 }
 
 /*--------------------------------------------------------------------*/
-
 int
 mgt_vcc_default(const char *b_arg, const char *f_arg, char *vcl, int C_flag)
 {
@@ -389,7 +396,7 @@ mgt_vcc_default(const char *b_arg, const char *f_arg, char *vcl, int C_flag)
 	}
 	strcpy(buf, "boot");
 
-	vf = mgt_VccCompile(&sb, vcl, C_flag);
+	(void)mgt_VccCompile(&sb, vcl, C_flag, &vf);
 	free(vcl);
 	if (VSB_len(sb) > 0)
 		fprintf(stderr, "%s", VSB_data(sb));
@@ -481,6 +488,7 @@ mcf_config_inline(struct cli *cli, const char * const *av, void *priv)
 	struct vsb *sb;
 	unsigned status;
 	struct vclprog *vp;
+	int i;
 
 	(void)priv;
 
@@ -491,13 +499,14 @@ mcf_config_inline(struct cli *cli, const char * const *av, void *priv)
 		return;
 	}
 
-	vf = mgt_VccCompile(&sb, av[3], 0);
+	i = mgt_VccCompile(&sb, av[3], 0, &vf);
 	if (VSB_len(sb) > 0)
 		VCLI_Out(cli, "%s\n", VSB_data(sb));
 	VSB_delete(sb);
-	if (vf == NULL) {
+	if (i) {
+		AZ(vf);
 		VCLI_Out(cli, "VCL compilation failed");
-		VCLI_SetResult(cli, CLIS_PARAM);
+		VCLI_SetResult(cli, (i < -1 ? CLIS_INTERROR : CLIS_PARAM));
 		return;
 	}
 	VCLI_Out(cli, "VCL compiled.");
@@ -519,6 +528,7 @@ mcf_config_load(struct cli *cli, const char * const *av, void *priv)
 	unsigned status;
 	char *p = NULL;
 	struct vclprog *vp;
+	int i;
 
 	(void)priv;
 	vp = mgt_vcc_byname(av[2]);
@@ -535,15 +545,16 @@ mcf_config_load(struct cli *cli, const char * const *av, void *priv)
 		return;
 	}
 
-	vf = mgt_VccCompile(&sb, vcl, 0);
+	i = mgt_VccCompile(&sb, vcl, 0, &vf);
 	free(vcl);
 
 	if (VSB_len(sb) > 0)
 		VCLI_Out(cli, "%s", VSB_data(sb));
 	VSB_delete(sb);
-	if (vf == NULL) {
+	if (i) {
+		AZ(vf);
 		VCLI_Out(cli, "VCL compilation failed");
-		VCLI_SetResult(cli, CLIS_PARAM);
+		VCLI_SetResult(cli, (i < -1 ? CLIS_INTERROR : CLIS_PARAM));
 		return;
 	}
 	VCLI_Out(cli, "VCL compiled.");
diff --git a/include/vcli.h b/include/vcli.h
index 04ac11c..21bfa26 100644
--- a/include/vcli.h
+++ b/include/vcli.h
@@ -202,7 +202,8 @@ enum VCLI_status_e {
 	CLIS_TRUNCATED	= 201,
 	CLIS_CANT	= 300,
 	CLIS_COMMS	= 400,
-	CLIS_CLOSE	= 500
+	CLIS_CLOSE	= 500,
+	CLIS_INTERROR	= 503
 };
 
 /* Length of first line of response */
diff --git a/include/vsub.h b/include/vsub.h
index 6705b11..ae89be5 100644
--- a/include/vsub.h
+++ b/include/vsub.h
@@ -28,7 +28,7 @@
  *
  */
 
-/* from libvarnish/subproc.c */
+/* from libvarnish/vsub.c */
 typedef void vsub_func_f(void*);
 
 int VSUB_run(struct vsb *sb, vsub_func_f *func, void *priv, const char *name,
diff --git a/lib/libvarnish/vsub.c b/lib/libvarnish/vsub.c
index 497119c..3550259 100644
--- a/lib/libvarnish/vsub.c
+++ b/lib/libvarnish/vsub.c
@@ -64,6 +64,13 @@ vsub_vlu(void *priv, const char *str)
 	return (0);
 }
 
+/* Fork and execute a function
+   Returns:
+     -2: Critical error (syscall failure or exit on signal)
+     -1: Non-zero return value
+      0: Success (zero exit value)
+*/
+
 int
 VSUB_run(struct vsb *sb, vsub_func_f *func, void *priv, const char *name,
     int maxlines)
@@ -81,7 +88,7 @@ VSUB_run(struct vsb *sb, vsub_func_f *func, void *priv, const char *name,
 	if (pipe(p) < 0) {
 		VSB_printf(sb, "Starting %s: pipe() failed: %s",
 		    name, strerror(errno));
-		return (-1);
+		return (-2);
 	}
 	assert(p[0] > STDERR_FILENO);
 	assert(p[1] > STDERR_FILENO);
@@ -90,7 +97,7 @@ VSUB_run(struct vsb *sb, vsub_func_f *func, void *priv, const char *name,
 		    name, strerror(errno));
 		AZ(close(p[0]));
 		AZ(close(p[1]));
-		return (-1);
+		return (-2);
 	}
 	if (pid == 0) {
 		AZ(close(STDIN_FILENO));
@@ -117,7 +124,7 @@ VSUB_run(struct vsb *sb, vsub_func_f *func, void *priv, const char *name,
 		if (rv < 0 && errno != EINTR) {
 			VSB_printf(sb, "Running %s: waitpid() failed: %s\n",
 			    name, strerror(errno));
-			return (-1);
+			return (-2);
 		}
 	} while (rv < 0);
 	if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
@@ -129,7 +136,7 @@ VSUB_run(struct vsb *sb, vsub_func_f *func, void *priv, const char *name,
 		if (WCOREDUMP(status))
 			VSB_printf(sb, ", core dumped");
 		VSB_printf(sb, "\n");
-		return (-1);
+		return (WIFSIGNALED(status) ? -2 : -1);
 	}
 	return (0);
 }
-- 
1.7.4.1




More information about the varnish-dev mailing list