[master] a810eb842 Refactor argv processing.
Poul-Henning Kamp
phk at FreeBSD.org
Wed Feb 16 14:18:05 UTC 2022
commit a810eb842c5f0ac46ecaeb4984135539df1dde2e
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Wed Feb 16 14:15:59 2022 +0000
Refactor argv processing.
Create a general list for holding "args to be dealt with later"
and use it for pretty much everything in that class.
Side-effect: Multiple -T, -M and -P options are supported now.
diff --git a/bin/varnishd/mgt/mgt_main.c b/bin/varnishd/mgt/mgt_main.c
index 4f65216ce..b49bebb09 100644
--- a/bin/varnishd/mgt/mgt_main.c
+++ b/bin/varnishd/mgt/mgt_main.c
@@ -50,7 +50,6 @@
#include "hash/hash_slinger.h"
#include "libvcc.h"
-#include "vav.h"
#include "vcli_serve.h"
#include "vend.h"
#include "vev.h"
@@ -73,16 +72,10 @@ struct VSC_mgt *VSC_C_mgt;
static int I_fd = -1;
static char *workdir;
-static struct vpf_fh *pfh1 = NULL;
-static struct vpf_fh *pfh2 = NULL;
-
static struct vfil_path *vcl_path = NULL;
-static VTAILQ_HEAD(,f_arg) f_args = VTAILQ_HEAD_INITIALIZER(f_args);
static const char opt_spec[] = "?a:b:Cdf:Fh:i:I:j:l:M:n:P:p:r:S:s:T:t:VW:x:";
-int optreset; // Some has it, some doesn't. Cheaper than auto*
-
/*--------------------------------------------------------------------*/
static void
@@ -175,6 +168,46 @@ usage(void)
/*--------------------------------------------------------------------*/
+struct arg_list {
+ unsigned magic;
+#define ARG_LIST_MAGIC 0x7b0d1bc4
+ char arg[2];
+ char *val;
+ VTAILQ_ENTRY(arg_list) list;
+ void *priv;
+};
+
+static VTAILQ_HEAD(, arg_list) arglist = VTAILQ_HEAD_INITIALIZER(arglist);
+
+static struct arg_list *
+arg_list_add(const char arg, const char *val)
+{
+ struct arg_list *alp;
+
+ ALLOC_OBJ(alp, ARG_LIST_MAGIC);
+ AN(alp);
+ alp->arg[0] = arg;
+ alp->arg[1] = '\0';
+ REPLACE(alp->val, val);
+ VTAILQ_INSERT_TAIL(&arglist, alp, list);
+ return (alp);
+}
+
+static unsigned
+arg_list_count(const char *arg)
+{
+ unsigned retval = 0;
+ struct arg_list *alp;
+
+ VTAILQ_FOREACH(alp, &arglist, list) {
+ if (!strcmp(alp->arg, arg))
+ retval++;
+ }
+ return (retval);
+}
+
+/*--------------------------------------------------------------------*/
+
static void
cli_check(const struct cli *cli)
{
@@ -252,7 +285,6 @@ mgt_Cflag_atexit(void)
if (getpid() != heritage.mgt_pid)
return;
VJ_rmdir("vmod_cache");
- VJ_unlink("_.pid");
(void)chdir("/");
VJ_rmdir(workdir);
}
@@ -411,10 +443,9 @@ struct f_arg {
#define F_ARG_MAGIC 0x840649a8
char *farg;
char *src;
- VTAILQ_ENTRY(f_arg) list;
};
-static void
+static struct f_arg *
mgt_f_read(const char *fn)
{
struct f_arg *fa;
@@ -431,7 +462,7 @@ mgt_f_read(const char *fn)
free(fa->farg);
fa->farg = fnp;
fa->src = f;
- VTAILQ_INSERT_TAIL(&f_args, fa, list);
+ return (fa);
}
static void
@@ -457,35 +488,34 @@ mgt_b_conv(const char *b_arg)
fa->src = strdup(VSB_data(vsb));
AN(fa->src);
VSB_destroy(&vsb);
- VTAILQ_INSERT_TAIL(&f_args, fa, list);
+ AZ(arg_list_count("f"));
+ arg_list_add('f', "")->priv = fa;
}
static int
-mgt_process_f_args(struct cli *cli, unsigned C_flag)
+mgt_process_f_arg(struct cli *cli, unsigned C_flag, void **fap)
{
int retval = 0;
struct f_arg *fa;
-
- while (!VTAILQ_EMPTY(&f_args)) {
- fa = VTAILQ_FIRST(&f_args);
- CHECK_OBJ_NOTNULL(fa, F_ARG_MAGIC);
- VTAILQ_REMOVE(&f_args, fa, list);
- mgt_vcl_startup(cli, fa->src,
- VTAILQ_EMPTY(&f_args) ? "boot" : NULL, fa->farg, C_flag);
- if (C_flag) {
- if (cli->result != CLIS_OK &&
- cli->result != CLIS_TRUNCATED)
- retval = 2;
- AZ(VSB_finish(cli->sb));
- fprintf(stderr, "%s\n", VSB_data(cli->sb));
- VSB_clear(cli->sb);
- } else {
- cli_check(cli);
- }
- free(fa->farg);
- free(fa->src);
- FREE_OBJ(fa);
+ const char *name = NULL;
+
+ TAKE_OBJ_NOTNULL(fa, fap, F_ARG_MAGIC);
+ if (arg_list_count("f") == 1)
+ name = "boot";
+ mgt_vcl_startup(cli, fa->src, name, fa->farg, C_flag);
+ if (C_flag) {
+ if (cli->result != CLIS_OK &&
+ cli->result != CLIS_TRUNCATED)
+ retval = 2;
+ AZ(VSB_finish(cli->sb));
+ fprintf(stderr, "%s\n", VSB_data(cli->sb));
+ VSB_clear(cli->sb);
+ } else {
+ cli_check(cli);
}
+ free(fa->farg);
+ free(fa->src);
+ FREE_OBJ(fa);
return (retval);
}
@@ -543,21 +573,15 @@ main(int argc, char * const *argv)
{
int o, eric_fd = -1;
unsigned C_flag = 0;
- unsigned f_flag = 0;
unsigned F_flag = 0;
const char *b_arg = NULL;
const char *i_arg = NULL;
const char *j_arg = NULL;
const char *h_arg = "critbit";
- const char *M_arg = NULL;
const char *n_arg = NULL;
- const char *P_arg = NULL;
const char *S_arg = NULL;
const char *s_arg = "default,100m";
const char *W_arg = NULL;
- int s_arg_given = 0;
- int novcl = 0;
- const char *T_arg = "localhost:0";
char *p;
struct cli cli[1];
const char *err;
@@ -565,6 +589,8 @@ main(int argc, char * const *argv)
struct sigaction sac;
struct vev *e;
pid_t pid;
+ struct arg_list *alp;
+ int first_arg = 1;
if (argc == 2 && !strcmp(argv[1], "--optstring")) {
printf("%s\n", opt_spec);
@@ -581,35 +607,22 @@ main(int argc, char * const *argv)
mgt_initialize(cli);
- /* Check if first argument is a special flag */
-
- o = getopt(argc, argv, opt_spec);
- switch (o) {
- case 'x':
- if (argc != 3)
- ARGV_ERR("Too many arguments for -x\n");
- mgt_x_arg(optarg);
- exit(0);
- case 'V':
- if (argc != 2)
- ARGV_ERR("Too many arguments for -V\n");
- VCS_Message("varnishd");
- exit(0);
- default:
- break;
- }
-
- /* First pass over arguments to determine overall configuration */
-
- do {
+ for (; (o = getopt(argc, argv, opt_spec)) != -1; first_arg = 0) {
switch (o) {
- case '?':
- usage();
- exit(2);
case 'V':
+ if (!first_arg)
+ ARGV_ERR("-V must be the first argument\n");
+ if (argc != 2)
+ ARGV_ERR("Too many arguments for -V\n");
+ VCS_Message("varnishd");
+ exit(0);
case 'x':
- ARGV_ERR("-%c must be the first argument\n", o);
- break;
+ if (!first_arg)
+ ARGV_ERR("-x must be the first argument\n");
+ if (argc != 3)
+ ARGV_ERR("Too many arguments for -x\n");
+ mgt_x_arg(optarg);
+ exit(0);
case 'b':
b_arg = optarg;
break;
@@ -619,31 +632,60 @@ main(int argc, char * const *argv)
case 'd':
d_flag++;
break;
- case 'f':
- f_flag = 1;
- break;
case 'F':
F_flag = 1;
break;
case 'j':
j_arg = optarg;
break;
- default:
+ case 'h':
+ h_arg = optarg;
+ break;
+ case 'i':
+ i_arg = optarg;
+ break;
+ case 'l':
+ arg_list_add('p', "vsl_space")->priv = optarg;
+ break;
+ case 'n':
+ n_arg = optarg;
+ break;
+ case 'S':
+ S_arg = optarg;
+ break;
+ case 't':
+ arg_list_add('p', "default_ttl")->priv = optarg;
+ break;
+ case 'W':
+ W_arg = optarg;
+ break;
+ case 'a':
+ case 'f':
+ case 'I':
+ case 'p':
+ case 'P':
+ case 'M':
+ case 'r':
+ case 's':
+ case 'T':
+ (void)arg_list_add(o, optarg);
break;
+ default:
+ usage();
+ exit(2);
}
- o = getopt(argc, argv, opt_spec);
- } while (o != -1);
+ }
if (argc != optind)
ARGV_ERR("Too many arguments (%s...)\n", argv[optind]);
- if (b_arg != NULL && f_flag)
+ if (b_arg != NULL && arg_list_count("f"))
ARGV_ERR("Only one of -b or -f can be specified\n");
if (d_flag && F_flag)
ARGV_ERR("Only one of -d or -F can be specified\n");
- if (C_flag && b_arg == NULL && !f_flag)
+ if (C_flag && b_arg == NULL && !arg_list_count("f"))
ARGV_ERR("-C needs either -b <backend> or -f <vcl_file>\n");
if (d_flag && C_flag)
@@ -652,12 +694,18 @@ main(int argc, char * const *argv)
if (F_flag && C_flag)
ARGV_ERR("-F makes no sense with -C\n");
- if (!d_flag && b_arg == NULL && !f_flag)
+ if (!d_flag && b_arg == NULL && !arg_list_count("f"))
ARGV_ERR("Neither -b nor -f given. (use -f '' to override)\n");
+ if (arg_list_count("I") > 1)
+ ARGV_ERR("\tOnly one -I allowed\n");
+
if (d_flag || F_flag)
complain_to_stderr = 1;
+ if (!arg_list_count("T"))
+ (void)arg_list_add('T', "localhost:0");
+
/*
* Start out by closing all unwanted file descriptors we might
* have inherited from sloppy process control daemons.
@@ -694,101 +742,46 @@ main(int argc, char * const *argv)
if (b_arg != NULL)
mgt_b_conv(b_arg);
- optind = 1;
- optreset = 1;
- while ((o = getopt(argc, argv, opt_spec)) != -1) {
- switch (o) {
- case 'C':
- case 'd':
- case 'F':
- case 'j':
- /* Handled in first pass */
- break;
+ /* Process delayed arguments */
+ VTAILQ_FOREACH(alp, &arglist, list) {
+ switch(alp->arg[0]) {
case 'a':
- MAC_Arg(optarg);
- break;
- case 'b':
- /* already dealt with */
+ MAC_Arg(alp->val);
break;
case 'f':
- if (*optarg == '\0')
- novcl = 1;
- else
- mgt_f_read(optarg);
- break;
- case 'h':
- h_arg = optarg;
- break;
- case 'i':
- i_arg = optarg;
+ if (*alp->val != '\0')
+ alp->priv = mgt_f_read(alp->val);
break;
case 'I':
- if (I_fd >= 0)
- ARGV_ERR("\tOnly one -I allowed\n");
VJ_master(JAIL_MASTER_FILE);
- I_fd = open(optarg, O_RDONLY);
+ I_fd = open(alp->val, O_RDONLY);
if (I_fd < 0)
ARGV_ERR("\tCan't open %s: %s\n",
- optarg, VAS_errtxt(errno));
+ alp->val, VAS_errtxt(errno));
VJ_master(JAIL_MASTER_LOW);
break;
- case 'l':
- MCF_ParamSet(cli, "vsl_space", optarg);
- cli_check(cli);
- break;
- case 'M':
- M_arg = optarg;
- break;
- case 'n':
- n_arg = optarg;
- break;
- case 'P':
- P_arg = optarg;
- break;
case 'p':
- p = strchr(optarg, '=');
- if (p == NULL)
- ARGV_ERR("\t-p lacks '='\n");
- AN(p);
- *p++ = '\0';
- MCF_ParamSet(cli, optarg, p);
- *--p = '=';
- cli_check(cli);
+ if (alp->priv) {
+ MCF_ParamSet(cli, alp->val, alp->priv);
+ } else {
+ p = strchr(alp->val, '=');
+ if (p == NULL)
+ ARGV_ERR("\t-p lacks '='\n");
+ AN(p);
+ *p++ = '\0';
+ MCF_ParamSet(cli, alp->val, p);
+ }
break;
case 'r':
- MCF_ParamProtect(cli, optarg);
- cli_check(cli);
- break;
- case 'S':
- S_arg = optarg;
+ MCF_ParamProtect(cli, alp->val);
break;
case 's':
- s_arg_given = 1;
- STV_Config(optarg);
- break;
- case 'T':
- if (!strcmp(optarg, "none"))
- T_arg = NULL;
- else
- T_arg = optarg;
- break;
- case 't':
- MCF_ParamSet(cli, "default_ttl", optarg);
- break;
- case 'W':
- W_arg = optarg;
+ STV_Config(alp->val);
break;
default:
- WRONG("Error in argument parsing");
+ break;
}
- }
- assert(argc == optind);
-
- /* XXX: we can have multiple CLI actions above, is this enough ? */
- if (cli[0].result != CLIS_OK) {
- AZ(VSB_finish(cli[0].sb));
- ARGV_ERR("Failed parameter creation:\n%s\n",
- VSB_data(cli[0].sb));
+ cli_check(cli);
}
VCLS_SetLimit(mgt_cls, &mgt_param.cli_limit);
@@ -839,13 +832,8 @@ main(int argc, char * const *argv)
if (C_flag)
AZ(atexit(mgt_Cflag_atexit));
- pfh1 = create_pid_file(&pid, "%s/_.pid", workdir);
-
- if (P_arg)
- pfh2 = create_pid_file(&pid, "%s", P_arg);
-
/* If no -s argument specified, process default -s argument */
- if (!s_arg_given)
+ if (!arg_list_count("s"))
STV_Config(s_arg);
/* Configure Transient storage, if user did not */
@@ -853,10 +841,23 @@ main(int argc, char * const *argv)
mgt_vcl_init();
- u = mgt_process_f_args(cli, C_flag);
+ u = 0;
+ VTAILQ_FOREACH(alp, &arglist, list) {
+ if (!strcmp(alp->arg, "f") && alp->priv != NULL)
+ u |= mgt_process_f_arg(cli, C_flag, &alp->priv);
+ }
if (C_flag)
exit(u);
+ VTAILQ_FOREACH(alp, &arglist, list) {
+ if (!strcmp(alp->arg, "P"))
+ alp->priv = create_pid_file(&pid, "%s", alp->val);
+ }
+
+ /* Implict -P argument */
+ alp = arg_list_add('P', NULL);
+ alp->priv = create_pid_file(&pid, "%s/_.pid", workdir);
+
if (VTAILQ_EMPTY(&heritage.socks))
MAC_Arg(":80\0"); // XXX: extra NUL for FlexeLint
@@ -871,10 +872,14 @@ main(int argc, char * const *argv)
mgt_SHM_static_alloc(i_arg, strlen(i_arg) + 1L, "Arg", "-i");
VSC_C_mgt = VSC_mgt_New(NULL, NULL, "");
- if (M_arg != NULL)
- mgt_cli_master(M_arg);
- if (T_arg != NULL)
- mgt_cli_telnet(T_arg);
+ VTAILQ_FOREACH(alp, &arglist, list) {
+ if (!strcmp(alp->arg, "M"))
+ mgt_cli_master(alp->val);
+ else if (!strcmp(alp->arg, "T") && strcmp(alp->val, "none"))
+ mgt_cli_telnet(alp->val);
+ else if (!strcmp(alp->arg, "P"))
+ VPF_Write(alp->priv);
+ }
AZ(VSB_finish(vident));
@@ -882,10 +887,6 @@ main(int argc, char * const *argv)
S_arg = make_secret(workdir);
AN(S_arg);
- VPF_Write(pfh1);
- if (pfh2 != NULL)
- VPF_Write(pfh2);
-
MGT_Complain(C_DEBUG, "Version: %s", VCS_String("V"));
MGT_Complain(C_DEBUG, "Platform: %s", VSB_data(vident) + 1);
@@ -918,7 +919,7 @@ main(int argc, char * const *argv)
assert(I_fd == -1);
err = mgt_has_vcl();
- if (!d_flag && err != NULL && !novcl)
+ if (!d_flag && err != NULL && !arg_list_count("f"))
MGT_Complain(C_ERR, "%s", err);
if (err == NULL && !d_flag)
@@ -967,8 +968,9 @@ main(int argc, char * const *argv)
MGT_Complain(C_INFO, "manager dies");
mgt_cli_close_all();
VEV_Destroy(&mgt_evb);
- VPF_Remove(pfh1);
- if (pfh2 != NULL)
- VPF_Remove(pfh2);
+ VTAILQ_FOREACH(alp, &arglist, list) {
+ if (!strcmp(alp->arg, "P"))
+ VPF_Remove(alp->priv);
+ }
exit(exit_status);
}
More information about the varnish-commit
mailing list