Mailing List Archive

Author: des
Date: 2007-06-18 09:31:50 +0200 (Mon, 18 Jun 2007)
New Revision: 1532

Modified:
trunk/varnish-cache/bin/varnishd/mgt_param.c
trunk/varnish-cache/bin/varnishd/varnishd.c
Log:
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


Modified: trunk/varnish-cache/bin/varnishd/mgt_param.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/mgt_param.c 2007-06-17 15:10:08 UTC (rev 1531)
+++ trunk/varnish-cache/bin/varnishd/mgt_param.c 2007-06-18 07:31:50 UTC (rev 1532)
@@ -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 @@

/*--------------------------------------------------------------------*/

+#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)
+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.-";
+ 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 @@
"naming conventions. Makes it possible to run "
"multiple varnishd instances on one server.\n"
EXPERIMENTAL,
- "", "hostname" },
+ "" },
{ NULL, NULL, NULL }
};


Modified: trunk/varnish-cache/bin/varnishd/varnishd.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/varnishd.c 2007-06-17 15:10:08 UTC (rev 1531)
+++ trunk/varnish-cache/bin/varnishd/varnishd.c 2007-06-18 07:31:50 UTC (rev 1532)
@@ -411,10 +411,7 @@
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 @@
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 @@
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();