[master] 33e6fe7 varnishd: streamline exit codes, fix exit codes for vsubs

Nils Goroll nils.goroll at uplex.de
Thu Aug 28 12:42:27 CEST 2014


commit 33e6fe71743059b912d2b00cdbc08196b003e440
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Thu Aug 28 12:32:08 2014 +0200

    varnishd: streamline exit codes, fix exit codes for vsubs
    
    This should bring us closer to the exit codes now documented in
    varnishd.rst
    
    Fixes #1572 for varnishd

diff --git a/bin/varnishd/mgt/mgt.h b/bin/varnishd/mgt/mgt.h
index ecdc6d8..4f4a8f6 100644
--- a/bin/varnishd/mgt/mgt.h
+++ b/bin/varnishd/mgt/mgt.h
@@ -117,7 +117,7 @@ void STV_Config_Transient(void);
 
 /* mgt_vcc.c */
 void mgt_vcc_init(void);
-int mgt_vcc_default(const char *bflag, const char *f_arg, char *vcl, int Cflag);
+unsigned mgt_vcc_default(const char *bflag, const char *f_arg, char *vcl, int Cflag);
 int mgt_push_vcls_and_start(unsigned *status, char **p);
 int mgt_has_vcl(void);
 extern char *mgt_cc_cmd;
diff --git a/bin/varnishd/mgt/mgt_child.c b/bin/varnishd/mgt/mgt_child.c
index 1ae0c0e..1fe8473 100644
--- a/bin/varnishd/mgt/mgt_child.c
+++ b/bin/varnishd/mgt/mgt_child.c
@@ -425,7 +425,7 @@ mgt_launch_child(struct cli *cli)
 
 		child_main();
 
-		exit(1);
+		exit(0);
 	}
 	assert(pid > 1);
 	REPORT(LOG_NOTICE, "child (%jd) Started", (intmax_t)pid);
@@ -683,7 +683,7 @@ mgt_sigint(const struct vev *e, int what)
 	(void)fflush(stdout);
 	if (child_pid >= 0)
 		mgt_stop_child();
-	exit (2);
+	exit(0);
 }
 
 /*--------------------------------------------------------------------*/
@@ -753,6 +753,7 @@ MGT_Run(void)
 	else if (!d_flag) {
 		mgt_launch_child(NULL);
 		if (child_state == CH_STOPPED) {
+			// XXX correct? or 0?
 			exit_status = 2;
 			return;
 		}
diff --git a/bin/varnishd/mgt/mgt_cli.c b/bin/varnishd/mgt/mgt_cli.c
index e7d7aa2..21fecfb 100644
--- a/bin/varnishd/mgt/mgt_cli.c
+++ b/bin/varnishd/mgt/mgt_cli.c
@@ -499,17 +499,17 @@ mgt_cli_secret(const char *S_arg)
 	fd = open(S_arg, O_RDONLY);
 	if (fd < 0) {
 		fprintf(stderr, "Can not open secret-file \"%s\"\n", S_arg);
-		exit (2);
+		exit(2);
 	}
 	mgt_got_fd(fd);
 	i = read(fd, buf, sizeof buf);
 	if (i == 0) {
 		fprintf(stderr, "Empty secret-file \"%s\"\n", S_arg);
-		exit (2);
+		exit(2);
 	}
 	if (i < 0) {
 		fprintf(stderr, "Can not read secret-file \"%s\"\n", S_arg);
-		exit (2);
+		exit(2);
 	}
 	AZ(close(fd));
 	secret_file = S_arg;
@@ -644,7 +644,7 @@ mgt_cli_master(const char *M_arg)
 	M_nta = VSS_resolve(M_arg, NULL, &M_ta);
 	if (M_nta <= 0) {
 		fprintf(stderr, "Could resolve -M argument to address\n");
-		exit (1);
+		exit(2);
 	}
 	M_nxt = 0;
 	AZ(M_poker);
diff --git a/bin/varnishd/mgt/mgt_main.c b/bin/varnishd/mgt/mgt_main.c
index 76b702b..ea7f950 100644
--- a/bin/varnishd/mgt/mgt_main.c
+++ b/bin/varnishd/mgt/mgt_main.c
@@ -199,7 +199,7 @@ cli_check(const struct cli *cli)
 	}
 	AZ(VSB_finish(cli->sb));
 	fprintf(stderr, "Error:\n%s\n", VSB_data(cli->sb));
-	exit (2);
+	exit(2);
 }
 
 /*--------------------------------------------------------------------
@@ -559,11 +559,11 @@ main(int argc, char * const *argv)
 		case 'x':
 			if (!strcmp(optarg, "dumprstparam")) {
 				MCF_DumpRstParam();
-				exit (0);
+				exit(0);
 			}
 			if (!strcmp(optarg, "dumprstvsl")) {
 				mgt_DumpRstVsl();
-				exit (0);
+				exit(0);
 			}
 			usage();
 			break;
@@ -651,11 +651,11 @@ main(int argc, char * const *argv)
 		    P_arg, strerror(errno));
 
 	if (b_arg != NULL || f_arg != NULL)
-		if (mgt_vcc_default(b_arg, f_arg, vcl, C_flag))
-			exit (2);
+		if ((o = mgt_vcc_default(b_arg, f_arg, vcl, C_flag)) != 0)
+			exit(o);
 
 	if (C_flag)
-		exit (0);
+		exit(0);
 
 	if (!d_flag) {
 		if (MGT_open_sockets())
diff --git a/bin/varnishd/mgt/mgt_param.c b/bin/varnishd/mgt/mgt_param.c
index 500916e..abfd86a 100644
--- a/bin/varnishd/mgt/mgt_param.c
+++ b/bin/varnishd/mgt/mgt_param.c
@@ -175,7 +175,7 @@ mcf_wrap(struct cli *cli, const char *text)
 			if (r == NULL) {
 				fprintf(stderr,
 				    "LINE with just one TAB: <%s>\n", text);
-				exit(2);
+				exit(4);
 			}
 			if (r - q > tw)
 				tw = r - q;
@@ -385,11 +385,11 @@ MCF_AddParams(struct parspec *ps)
 		if (isspace(s[-1])) {
 			fprintf(stderr,
 			    "Param->descr has trailing space: %s\n", pp->name);
-			exit(2);
+			exit(4);
 		}
 		if (mcf_findpar(pp->name) != NULL) {
 			fprintf(stderr, "Duplicate param: %s\n", pp->name);
-			exit(2);
+			exit(4);
 		}
 		if (strlen(pp->name) + 1 > margin2)
 			margin2 = strlen(pp->name) + 1;
diff --git a/bin/varnishd/mgt/mgt_sandbox_solaris.c b/bin/varnishd/mgt/mgt_sandbox_solaris.c
index 9ead087..c664cc2 100644
--- a/bin/varnishd/mgt/mgt_sandbox_solaris.c
+++ b/bin/varnishd/mgt/mgt_sandbox_solaris.c
@@ -236,7 +236,7 @@ mgt_sandbox_solaris_add_inheritable(priv_set_t *pset, enum sandbox_e who)
 		break;
 	default:
 		REPORT(LOG_ERR, "INCOMPLETE AT: %s(%d)\n", __func__, __LINE__);
-		exit(1);
+		exit(4);
 	}
 }
 
@@ -263,7 +263,7 @@ mgt_sandbox_solaris_add_effective(priv_set_t *pset, enum sandbox_e who)
 		break;
 	default:
 		REPORT(LOG_ERR, "INCOMPLETE AT: %s(%d)\n", __func__, __LINE__);
-		exit(1);
+		exit(4);
 	}
 }
 
@@ -286,7 +286,7 @@ mgt_sandbox_solaris_add_permitted(priv_set_t *pset, enum sandbox_e who)
 		break;
 	default:
 		REPORT(LOG_ERR, "INCOMPLETE AT: %s(%d)\n", __func__, __LINE__);
-		exit(1);
+		exit(4);
 	}
 }
 
diff --git a/bin/varnishd/mgt/mgt_shmem.c b/bin/varnishd/mgt/mgt_shmem.c
index 130b48f..ed020f7 100644
--- a/bin/varnishd/mgt/mgt_shmem.c
+++ b/bin/varnishd/mgt/mgt_shmem.c
@@ -100,7 +100,7 @@ vsm_n_check(void)
 	struct stat st;
 	pid_t pid;
 	struct VSM_head vsmh;
-	int retval = 2;
+	int retval = 1;
 
 	fd = open(VSM_FILENAME, O_RDWR, 0644);
 	if (fd < 0)
@@ -217,7 +217,7 @@ mgt_SHM_Create(void)
 
 	if (p == MAP_FAILED) {
 		fprintf(stderr, "Mmap error %s: %s\n", fnbuf, strerror(errno));
-		exit (-1);
+		exit(1);
 	}
 
 	mgt_vsm_p = p;
@@ -250,7 +250,7 @@ mgt_SHM_Create(void)
 		fprintf(stderr, "Rename failed %s -> %s: %s\n",
 		    fnbuf, VSM_FILENAME, strerror(errno));
 		(void)unlink(fnbuf);
-		exit (-1);
+		exit(1);
 	}
 
 #ifdef __OpenBSD__
@@ -325,7 +325,7 @@ mgt_SHM_Init(void)
 	/* Collision check with already running varnishd */
 	i = vsm_n_check();
 	if (i)
-		exit(i);
+		exit(2);
 
 	/* Create our static VSM instance */
 	static_vsm = VSM_common_new(static_vsm_buf, sizeof static_vsm_buf);
diff --git a/bin/varnishd/mgt/mgt_vcc.c b/bin/varnishd/mgt/mgt_vcc.c
index 4d97de6..b76b016 100644
--- a/bin/varnishd/mgt/mgt_vcc.c
+++ b/bin/varnishd/mgt/mgt_vcc.c
@@ -151,22 +151,22 @@ run_vcc(void *priv)
 		printf("%s", VSB_data(sb));
 	VSB_delete(sb);
 	if (csrc == NULL)
-		exit (1);
+		exit(2);
 
 	fd = open(vp->sf, O_WRONLY);
 	if (fd < 0) {
 		fprintf(stderr, "Cannot open %s", vp->sf);
-		exit (1);
+		exit(2);
 	}
 	l = strlen(csrc);
 	i = write(fd, csrc, l);
 	if (i != l) {
 		fprintf(stderr, "Cannot write %s", vp->sf);
-		exit (1);
+		exit(2);
 	}
 	AZ(close(fd));
 	free(csrc);
-	exit (0);
+	exit(0);
 }
 
 /*--------------------------------------------------------------------
@@ -228,7 +228,7 @@ run_dlopen(void *priv)
  */
 
 static char *
-mgt_run_cc(const char *vcl, struct vsb *sb, int C_flag)
+mgt_run_cc(const char *vcl, struct vsb *sb, int C_flag, unsigned *status)
 {
 	char *csrc;
 	struct vsb *cmdsb;
@@ -236,12 +236,16 @@ mgt_run_cc(const char *vcl, struct vsb *sb, int C_flag)
 	char of[sizeof sf + 1];
 	char *retval;
 	int sfd, i;
+	unsigned subs;
 	struct vcc_priv vp;
 
+	*status = 0;
+
 	/* Create temporary C source file */
 	sfd = VFIL_tmpfile(sf);
 	if (sfd < 0) {
 		VSB_printf(sb, "Failed to create %s: %s", sf, strerror(errno));
+		*status = 2;
 		return (NULL);
 	}
 	(void)fchown(sfd, mgt_param.uid, mgt_param.gid);
@@ -253,8 +257,10 @@ 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)) {
+	subs = VSUB_run(sb, run_vcc, &vp, "VCC-compiler", -1);
+	if (subs) {
 		(void)unlink(sf);
+		*status = subs;
 		return (NULL);
 	}
 
@@ -277,6 +283,7 @@ mgt_run_cc(const char *vcl, struct vsb *sb, int C_flag)
 		VSB_printf(sb, "Failed to create %s: %s",
 		    of, strerror(errno));
 		(void)unlink(sf);
+		*status = 2;
 		return (NULL);
 	}
 	(void)fchown(i, mgt_param.uid, mgt_param.gid);
@@ -286,24 +293,27 @@ mgt_run_cc(const char *vcl, struct vsb *sb, int C_flag)
 	cmdsb = mgt_make_cc_cmd(sf, of);
 
 	/* Run the C-compiler in a sub-shell */
-	i = VSUB_run(sb, run_cc, VSB_data(cmdsb), "C-compiler", 10);
+	subs = VSUB_run(sb, run_cc, VSB_data(cmdsb), "C-compiler", 10);
 
 	(void)unlink(sf);
 	VSB_delete(cmdsb);
 
-	if (!i)
-		i = VSUB_run(sb, run_dlopen, of, "dlopen", 10);
+	if (!subs)
+		subs = VSUB_run(sb, run_dlopen, of, "dlopen", 10);
 
 	/* Ensure the file is readable to the unprivileged user */
-	if (!i) {
+	if (!subs) {
 		i = chmod(of, 0755);
-		if (i)
+		if (i) {
 			VSB_printf(sb, "Failed to set permissions on %s: %s",
 			    of, strerror(errno));
+			subs = 2;
+		}
 	}
 
-	if (i) {
+	if (subs) {
 		(void)unlink(of);
+		*status = subs;
 		return (NULL);
 	}
 
@@ -315,13 +325,13 @@ mgt_run_cc(const char *vcl, struct vsb *sb, int C_flag)
 /*--------------------------------------------------------------------*/
 
 static char *
-mgt_VccCompile(struct vsb **sb, const char *b, int C_flag)
+mgt_VccCompile(struct vsb **sb, const char *b, int C_flag, unsigned *status)
 {
 	char *vf;
 
 	*sb = VSB_new_auto();
 	XXXAN(*sb);
-	vf = mgt_run_cc(b, *sb, C_flag);
+	vf = mgt_run_cc(b, *sb, C_flag, status);
 	AZ(VSB_finish(*sb));
 	return (vf);
 }
@@ -380,13 +390,14 @@ mgt_vcc_delbyname(const char *name)
 
 /*--------------------------------------------------------------------*/
 
-int
+unsigned
 mgt_vcc_default(const char *b_arg, const char *f_arg, char *vcl, int C_flag)
 {
 	char *vf;
 	struct vsb *sb;
 	struct vclprog *vp;
 	char buf[BUFSIZ];
+	unsigned status = 0;
 
 	/* XXX: annotate vcl with -b/-f arg so people know where it came from */
 	(void)f_arg;
@@ -411,7 +422,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);
+	vf = mgt_VccCompile(&sb, vcl, C_flag, &status);
 	free(vcl);
 	if (VSB_len(sb) > 0)
 		fprintf(stderr, "%s", VSB_data(sb));
@@ -419,13 +430,13 @@ mgt_vcc_default(const char *b_arg, const char *f_arg, char *vcl, int C_flag)
 	if (C_flag && vf != NULL)
 		AZ(unlink(vf));
 	if (vf == NULL) {
+		assert(status != 0);
 		fprintf(stderr, "\nVCL compilation failed\n");
-		return (1);
 	} else {
 		vp = mgt_vcc_add(buf, vf);
 		vp->active = 1;
-		return (0);
 	}
+	return (status);
 }
 
 /*--------------------------------------------------------------------*/
@@ -511,11 +522,12 @@ mcf_config_inline(struct cli *cli, const char * const *av, void *priv)
 		return;
 	}
 
-	vf = mgt_VccCompile(&sb, av[3], 0);
+	vf = mgt_VccCompile(&sb, av[3], 0, &status);
 	if (VSB_len(sb) > 0)
 		VCLI_Out(cli, "%s\n", VSB_data(sb));
 	VSB_delete(sb);
 	if (vf == NULL) {
+		assert(status != 0);
 		VCLI_Out(cli, "VCL compilation failed");
 		VCLI_SetResult(cli, CLIS_PARAM);
 		return;
@@ -536,7 +548,7 @@ mcf_config_load(struct cli *cli, const char * const *av, void *priv)
 {
 	char *vf, *vcl;
 	struct vsb *sb;
-	unsigned status;
+	unsigned status = 0;
 	char *p = NULL;
 	struct vclprog *vp;
 
@@ -555,13 +567,14 @@ mcf_config_load(struct cli *cli, const char * const *av, void *priv)
 		return;
 	}
 
-	vf = mgt_VccCompile(&sb, vcl, 0);
+	vf = mgt_VccCompile(&sb, vcl, 0, &status);
 	free(vcl);
 
 	if (VSB_len(sb) > 0)
 		VCLI_Out(cli, "%s", VSB_data(sb));
 	VSB_delete(sb);
 	if (vf == NULL) {
+		assert(status != 0);
 		VCLI_Out(cli, "VCL compilation failed");
 		VCLI_SetResult(cli, CLIS_PARAM);
 		return;
diff --git a/bin/varnishd/storage/storage_file.c b/bin/varnishd/storage/storage_file.c
index 3d0e006..863ad25 100644
--- a/bin/varnishd/storage/storage_file.c
+++ b/bin/varnishd/storage/storage_file.c
@@ -443,7 +443,7 @@ smf_open(const struct stevedore *st)
 
 	/* XXX */
 	if (sum < MINPAGES * (off_t)getpagesize())
-		exit (2);
+		exit(4);
 
 	sc->stats->g_space += sc->filesize;
 }
diff --git a/include/vsub.h b/include/vsub.h
index 6705b11..274663a 100644
--- a/include/vsub.h
+++ b/include/vsub.h
@@ -31,5 +31,5 @@
 /* from libvarnish/subproc.c */
 typedef void vsub_func_f(void*);
 
-int VSUB_run(struct vsb *sb, vsub_func_f *func, void *priv, const char *name,
+unsigned VSUB_run(struct vsb *sb, vsub_func_f *func, void *priv, const char *name,
     int maxlines);
diff --git a/lib/libvarnish/vsub.c b/lib/libvarnish/vsub.c
index 497119c..ffe93ab 100644
--- a/lib/libvarnish/vsub.c
+++ b/lib/libvarnish/vsub.c
@@ -64,7 +64,8 @@ vsub_vlu(void *priv, const char *str)
 	return (0);
 }
 
-int
+/* returns an exit code */
+unsigned
 VSUB_run(struct vsb *sb, vsub_func_f *func, void *priv, const char *name,
     int maxlines)
 {
@@ -81,7 +82,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 (1);
 	}
 	assert(p[0] > STDERR_FILENO);
 	assert(p[1] > STDERR_FILENO);
@@ -90,7 +91,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 (1);
 	}
 	if (pid == 0) {
 		AZ(close(STDIN_FILENO));
@@ -101,7 +102,12 @@ VSUB_run(struct vsb *sb, vsub_func_f *func, void *priv, const char *name,
 		for (sfd = STDERR_FILENO + 1; sfd < 100; sfd++)
 			(void)close(sfd);
 		func(priv);
-		_exit(1);
+		/*
+		 * func should either exec or exit, so getting here should be
+		 * treated like an assertion failure - except that we don't know
+		 * if it's save to trigger an acutal assertion
+		 */
+		_exit(4);
 	}
 	AZ(close(p[1]));
 	vlu = VLU_New(&sp, vsub_vlu, 0);
@@ -117,19 +123,19 @@ 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 (1);
 		}
 	} while (rv < 0);
 	if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
 		VSB_printf(sb, "Running %s failed", name);
 		if (WIFEXITED(status))
-			VSB_printf(sb, ", exit %d", WEXITSTATUS(status));
+			VSB_printf(sb, ", exited with %d", WEXITSTATUS(status));
 		if (WIFSIGNALED(status))
 			VSB_printf(sb, ", signal %d", WTERMSIG(status));
 		if (WCOREDUMP(status))
 			VSB_printf(sb, ", core dumped");
 		VSB_printf(sb, "\n");
-		return (-1);
+		return (WEXITSTATUS(status));
 	}
 	return (0);
 }
diff --git a/lib/libvarnish/vtim.c b/lib/libvarnish/vtim.c
index c0efdbd..50033c2 100644
--- a/lib/libvarnish/vtim.c
+++ b/lib/libvarnish/vtim.c
@@ -224,7 +224,7 @@ tst(const char *s, time_t good)
 	if (t != good) {
 		printf("Parse error! Got: %jd should have %jd diff %jd\n",
 		    (intmax_t)t, (intmax_t)good, (intmax_t)(t - good));
-		exit (2);
+		exit(4);
 	}
 }
 
@@ -267,7 +267,7 @@ tst_delta()
 
 	if (err) {
 		printf("%d time delta test errrors\n", err);
-		exit (2);
+		exit(4);
 	}
 }
 



More information about the varnish-commit mailing list