From 5fa63124e4a24642baeee9304fc12b10703ffee5 Mon Sep 17 00:00:00 2001 From: des Date: Mon, 18 Jun 2007 07:31:50 +0000 Subject: [PATCH] Further tweak_name() improvements: restructure to reduce indentation; simplify error handling; use a regexp to check the name syntax; check CLI errors after the getopt() loop. Discussed with: ceciliehf git-svn-id: svn+ssh://projects.linpro.no/svn/varnish/trunk@1532 d4fa192b-c00b-0410-8231-f00ffab90ce4 --- varnish-cache/bin/varnishd/mgt_param.c | 123 ++++++++++++------------- varnish-cache/bin/varnishd/varnishd.c | 16 ++-- 2 files changed, 67 insertions(+), 72 deletions(-) diff --git a/varnish-cache/bin/varnishd/mgt_param.c b/varnish-cache/bin/varnishd/mgt_param.c index bbb1a65c..ad7cb5b8 100644 --- a/varnish-cache/bin/varnishd/mgt_param.c +++ b/varnish-cache/bin/varnishd/mgt_param.c @@ -33,8 +33,9 @@ #include #include -#include #include +#include +#include #include #include #include @@ -498,74 +499,68 @@ tweak_ping_interval(struct cli *cli, struct parspec *par, const char *arg) /*--------------------------------------------------------------------*/ +#define NAME_RE "^([0-9A-Za-z-]+)(\\.[0-9A-Za-z-]+)*$" +static regex_t *name_re; + static void -tweak_name(struct cli *cli, struct parspec *par, const char* arg) -{ - struct stat st; - struct stat st_old; - char *path; - char *old_path; - int renaming; - char hostname[1024]; - char valid_chars[65] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" - "abcdefghijklmnopqrstuvwxyz" - "0123456789.-"; +tweak_name(struct cli *cli, struct parspec *par, const char *arg) +{ + char hostname[1024], path[1024]; + int error; + (void)par; - if (arg != NULL) { - if (strlen(arg) == 0) { - gethostname(hostname, sizeof hostname); + if (arg == NULL) { + cli_out(cli, "\"%s\"", master.name); + return; + } + + /* empty string -> hostname */ + if (*arg == '\0') { + if (gethostname(hostname, sizeof hostname) == 0) arg = hostname; + else + arg = "localhost"; + } + + /* check that the new name follows hostname convention */ + if (name_re == NULL) { + name_re = malloc(sizeof *name_re); + AN(name_re); + AZ(regcomp(name_re, NAME_RE, REG_EXTENDED|REG_NOSUB)); + } + if (regexec(name_re, arg, 0, NULL, 0) != 0) { + cli_out(cli, "Invalid instance name"); + cli_result(cli, CLIS_PARAM); + return; + } + + error = 0; + snprintf(path, sizeof path, "/tmp/%s", arg); /* XXX overflow */ + if (master.name && *master.name) { + struct stat old_st; + char old_path[1024]; + + /* rename old directory */ + snprintf(old_path, sizeof old_path, "/tmp/%s", master.name); /* XXX overflow */ + if (stat(old_path, &old_st) == 0 && S_ISDIR(old_st.st_mode)) { + error = rename(old_path, path); + } else { + error = (mkdir(path, 0755) != 0 && errno != EEXIST); } - /* Check that the new name follows hostname convention */ - if (strspn(arg, valid_chars) != strlen(arg)) { - cli_out(cli, "Error: %s is an invalid name\n", arg); - cli_result(cli, CLIS_PARAM); - return; - } - asprintf(&old_path, "/tmp/%s", master.name); - /* Create/rename the temporary varnish directory */ - asprintf(&path, "/tmp/%s", arg); - renaming = (master.name && !stat(old_path, &st_old) && - S_ISDIR(st_old.st_mode)); - if (stat(path, &st)) { - if (renaming) { - if (rename(old_path, path)) { - cli_out(cli, - "Error: Directory %s could not be " - "renamed to %s", - old_path, path); - cli_result(cli, CLIS_PARAM); - return; - } - } else { - if (mkdir(path, 0755)) { - fprintf(stderr, - "Error: Directory %s could not be created", - path); - exit (2); - } - } - } else if (renaming) { - cli_out(cli, "Error: Directory %s could not be " - "renamed to %s", old_path, path); - cli_result(cli, CLIS_PARAM); - return; - } - stat(path, &st); - /* /tmp/varnishname should now be a directory */ - if (!S_ISDIR(st.st_mode)) { - fprintf(stderr, - "Error: \"%s\" " - "is not a directory\n", path); - exit (2); - } - /* Everything is fine, store the (new) name */ - free(master.name); - master.name = strdup(arg); + } else { + error = (mkdir(path, 0755) != 0 && errno != EEXIST); } - else - cli_out(cli, "\"%s\"", master.name); + + if (error || chdir(path) != 0) { + cli_out(cli, "could not create %s: %s", path, strerror(errno)); + cli_result(cli, CLIS_CANT); + return; + } + + /* Everything is fine, store the (new) name */ + master.name = strdup(arg); + XXXAN(master.name); } /*--------------------------------------------------------------------*/ @@ -748,7 +743,7 @@ static struct parspec parspec[] = { "naming conventions. Makes it possible to run " "multiple varnishd instances on one server.\n" EXPERIMENTAL, - "", "hostname" }, + "" }, { NULL, NULL, NULL } }; diff --git a/varnish-cache/bin/varnishd/varnishd.c b/varnish-cache/bin/varnishd/varnishd.c index b34587e9..923fde26 100644 --- a/varnish-cache/bin/varnishd/varnishd.c +++ b/varnish-cache/bin/varnishd/varnishd.c @@ -411,10 +411,7 @@ main(int argc, char *argv[]) struct cli cli[1]; struct pidfh *pfh = NULL; char buf[BUFSIZ]; - char valid_chars[65] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" - "abcdefghijklmnopqrstuvwxyz" - "0123456789.-"; - + setbuf(stdout, NULL); setbuf(stderr, NULL); @@ -454,10 +451,6 @@ main(int argc, char *argv[]) h_arg = optarg; break; case 'n': - if (strspn(optarg, valid_chars) != strlen(optarg)) { - fprintf(stderr, "%s is not a valid name\n", optarg); - exit(1); - } MCF_ParamSet(cli, "name", optarg); break; case 'P': @@ -502,6 +495,13 @@ main(int argc, char *argv[]) usage(); } + if (cli[0].result != CLIS_OK) { + fprintf(stderr, "Parameter errors:\n"); + vsb_finish(cli[0].sb); + fprintf(stderr, "%s\n", vsb_data(cli[0].sb)); + exit(1); + } + if (b_arg != NULL && f_arg != NULL) { fprintf(stderr, "Only one of -b or -f can be specified\n"); usage(); -- 2.39.5