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

diff --git a/varnish-cache/bin/varnishd/mgt_param.c b/varnish-cache/bin/varnishd/mgt_param.c
index bbb1a65cfffd1303705609681c2e89df2924b83b..ad7cb5b84cb4f59f410cc59747b2b610e60c3308 100644 (file)
--- a/varnish-cache/bin/varnishd/mgt_param.c
+++ b/varnish-cache/bin/varnishd/mgt_param.c
@@ -33,8 +33,9 @@
 #include <sys/stat.h>
 
 #include <grp.h>
-#include <pwd.h>
 #include <limits.h>
+#include <pwd.h>
+#include <regex.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -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 b34587e92bfa8d0705768002097f7c33d8f61e7e..923fde26e01cc2e80512b7695603d52fa9b53fa2 100644 (file)
--- 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();