Mailing List Archive

Author: phk
Date: 2008-07-19 11:24:07 +0200 (Sat, 19 Jul 2008)
New Revision: 2955

Modified:
trunk/varnish-cache/bin/varnishd/cache.h
trunk/varnish-cache/bin/varnishd/cache_hash.c
trunk/varnish-cache/bin/varnishd/common.h
trunk/varnish-cache/bin/varnishd/flint.lnt
trunk/varnish-cache/bin/varnishd/hash_classic.c
trunk/varnish-cache/bin/varnishd/hash_simple_list.c
trunk/varnish-cache/bin/varnishd/hash_slinger.h
trunk/varnish-cache/bin/varnishd/mgt.h
trunk/varnish-cache/bin/varnishd/stevedore.c
trunk/varnish-cache/bin/varnishd/stevedore.h
trunk/varnish-cache/bin/varnishd/storage_file.c
trunk/varnish-cache/bin/varnishd/storage_malloc.c
trunk/varnish-cache/bin/varnishd/varnishd.c
Log:
Wash & cleaning of the -h and -s argument handling.

No functional changes.

Add ARGV_ERR() macro for reporting argument trouble.

Do selection of method in varnishd.c, using generic choice function.

Split subarguments into argc+argv at commans, and pass those down to
avoid repetitive and bogus string munging.

Catch more error conditions with respect to subarguments.

Add mini_obj magics to stevedores and slingers.

Generally tidy up along the way.


Modified: trunk/varnish-cache/bin/varnishd/cache.h
===================================================================
--- trunk/varnish-cache/bin/varnishd/cache.h 2008-07-19 07:19:12 UTC (rev 2954)
+++ trunk/varnish-cache/bin/varnishd/cache.h 2008-07-19 09:24:07 UTC (rev 2955)
@@ -464,7 +464,7 @@
int HSH_Compare(const struct sess *sp, const struct objhead *o);
void HSH_Copy(const struct sess *sp, const struct objhead *o);
struct object *HSH_Lookup(struct sess *sp);
-void HSH_Unbusy(struct sess *sp);
+void HSH_Unbusy(const struct sess *sp);
void HSH_Ref(struct object *o);
void HSH_Deref(struct object *o);
double HSH_Grace(double g);

Modified: trunk/varnish-cache/bin/varnishd/cache_hash.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/cache_hash.c 2008-07-19 07:19:12 UTC (rev 2954)
+++ trunk/varnish-cache/bin/varnishd/cache_hash.c 2008-07-19 09:24:07 UTC (rev 2955)
@@ -302,7 +302,7 @@
}

void
-HSH_Unbusy(struct sess *sp)
+HSH_Unbusy(const struct sess *sp)
{
struct object *o;
struct objhead *oh;

Modified: trunk/varnish-cache/bin/varnishd/common.h
===================================================================
--- trunk/varnish-cache/bin/varnishd/common.h 2008-07-19 07:19:12 UTC (rev 2954)
+++ trunk/varnish-cache/bin/varnishd/common.h 2008-07-19 09:24:07 UTC (rev 2955)
@@ -58,3 +58,9 @@

/* Really belongs in mgt.h, but storage_file chokes on both */
void mgt_child_inherit(int fd, const char *what);
+
+#define ARGV_ERR(...) \
+ do { \
+ fprintf(stderr, "Error: " __VA_ARGS__); \
+ exit(2); \
+ } while (0);

Modified: trunk/varnish-cache/bin/varnishd/flint.lnt
===================================================================
--- trunk/varnish-cache/bin/varnishd/flint.lnt 2008-07-19 07:19:12 UTC (rev 2954)
+++ trunk/varnish-cache/bin/varnishd/flint.lnt 2008-07-19 09:24:07 UTC (rev 2955)
@@ -59,6 +59,7 @@
-emacro(827, assert) // loop not reachable
-emacro(774, assert) // booelan always true
-emacro(774, HTTPH) // always false
+-emacro(527, ARGV_ERR) // unreachable

// cache.h
-emacro(506, INCOMPL) // Constant value Boolean

Modified: trunk/varnish-cache/bin/varnishd/hash_classic.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/hash_classic.c 2008-07-19 07:19:12 UTC (rev 2954)
+++ trunk/varnish-cache/bin/varnishd/hash_classic.c 2008-07-19 09:24:07 UTC (rev 2955)
@@ -68,15 +68,19 @@
* The ->init method allows the management process to pass arguments
*/

-static int
-hcl_init(const char *p)
+static void
+hcl_init(int ac, char * const *av)
{
int i;
unsigned u;

- i = sscanf(p, "%u", &u);
+ if (ac == 0)
+ return;
+ if (ac > 1)
+ ARGV_ERR("(-hclassic) too many arguments\n");
+ i = sscanf(av[0], "%u", &u);
if (i <= 0 || u == 0)
- return (0);
+ return;
if (u > 2 && !(u & (u - 1))) {
fprintf(stderr,
"NOTE:\n"
@@ -88,7 +92,7 @@
}
hcl_nhash = u;
fprintf(stderr, "Classic hash: %u buckets\n", hcl_nhash);
- return (0);
+ return;
}

/*--------------------------------------------------------------------
@@ -245,9 +249,10 @@
/*--------------------------------------------------------------------*/

struct hash_slinger hcl_slinger = {
- "classic",
- hcl_init,
- hcl_start,
- hcl_lookup,
- hcl_deref,
+ .magic = SLINGER_MAGIC,
+ .name = "classic",
+ .init = hcl_init,
+ .start = hcl_start,
+ .lookup = hcl_lookup,
+ .deref = hcl_deref,
};

Modified: trunk/varnish-cache/bin/varnishd/hash_simple_list.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/hash_simple_list.c 2008-07-19 07:19:12 UTC (rev 2954)
+++ trunk/varnish-cache/bin/varnishd/hash_simple_list.c 2008-07-19 09:24:07 UTC (rev 2955)
@@ -139,9 +139,9 @@
/*--------------------------------------------------------------------*/

struct hash_slinger hsl_slinger = {
- "simple_list",
- NULL,
- hsl_start,
- hsl_lookup,
- hsl_deref,
+ .magic = SLINGER_MAGIC,
+ .name = "simple",
+ .start = hsl_start,
+ .lookup = hsl_lookup,
+ .deref = hsl_deref,
};

Modified: trunk/varnish-cache/bin/varnishd/hash_slinger.h
===================================================================
--- trunk/varnish-cache/bin/varnishd/hash_slinger.h 2008-07-19 07:19:12 UTC (rev 2954)
+++ trunk/varnish-cache/bin/varnishd/hash_slinger.h 2008-07-19 09:24:07 UTC (rev 2955)
@@ -31,12 +31,14 @@

struct sess;

-typedef int hash_init_f(const char *);
+typedef void hash_init_f(int ac, char * const *av);
typedef void hash_start_f(void);
typedef struct objhead *hash_lookup_f(const struct sess *sp, struct objhead *nobj);
typedef int hash_deref_f(const struct objhead *obj);

struct hash_slinger {
+ unsigned magic;
+#define SLINGER_MAGIC 0x1b720cba
const char *name;
hash_init_f *init;
hash_start_f *start;

Modified: trunk/varnish-cache/bin/varnishd/mgt.h
===================================================================
--- trunk/varnish-cache/bin/varnishd/mgt.h 2008-07-19 07:19:12 UTC (rev 2954)
+++ trunk/varnish-cache/bin/varnishd/mgt.h 2008-07-19 09:24:07 UTC (rev 2955)
@@ -63,11 +63,7 @@
int mgt_has_vcl(void);
extern char *mgt_cc_cmd;

-#include "hash_slinger.h"

-extern struct hash_slinger hsl_slinger;
-extern struct hash_slinger hcl_slinger;
-
#define REPORT0(pri, fmt) \
do { \
fprintf(stderr, fmt "\n"); \

Modified: trunk/varnish-cache/bin/varnishd/stevedore.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/stevedore.c 2008-07-19 07:19:12 UTC (rev 2954)
+++ trunk/varnish-cache/bin/varnishd/stevedore.c 2008-07-19 09:24:07 UTC (rev 2955)
@@ -37,9 +37,6 @@
#include "cache.h"
#include "stevedore.h"

-extern struct stevedore sma_stevedore;
-extern struct stevedore smf_stevedore;
-
static VTAILQ_HEAD(, stevedore) stevedores =
VTAILQ_HEAD_INITIALIZER(stevedores);

@@ -95,47 +92,21 @@
st->stevedore->free(st);
}

-static int
-cmp_storage(const struct stevedore *s, const char *p, const char *q)
-{
- unsigned u;
-
- u = pdiff(p, q);
- if (strlen(s->name) != u)
- return (1);
- if (strncmp(s->name, p, u))
- return (1);
- return (0);
-}
-
void
-STV_add(const char *spec)
+STV_add(const struct stevedore *stv2, int ac, char * const *av)
{
- const char *p, *q;
struct stevedore *stv;

- p = strchr(spec, ',');
- if (p == NULL)
- q = p = strchr(spec, '\0');
- else
- q = p + 1;
- xxxassert(p != NULL);
- xxxassert(q != NULL);
-
- stv = malloc(sizeof *stv);
+ CHECK_OBJ_NOTNULL(stv2, STEVEDORE_MAGIC);
+ ALLOC_OBJ(stv, STEVEDORE_MAGIC);
AN(stv);

- if (!cmp_storage(&sma_stevedore, spec, p)) {
- *stv = sma_stevedore;
- } else if (!cmp_storage(&smf_stevedore, spec, p)) {
- *stv = smf_stevedore;
- } else {
- fprintf(stderr, "Unknown storage method \"%.*s\"\n",
- (int)(p - spec), spec);
- exit (2);
- }
+ *stv = *stv2;
+
if (stv->init != NULL)
- stv->init(stv, q);
+ stv->init(stv, ac, av);
+ else if (ac != 0)
+ ARGV_ERR("(-s%s) too many arguments\n", stv->name);

VTAILQ_INSERT_TAIL(&stevedores, stv, list);


Modified: trunk/varnish-cache/bin/varnishd/stevedore.h
===================================================================
--- trunk/varnish-cache/bin/varnishd/stevedore.h 2008-07-19 07:19:12 UTC (rev 2954)
+++ trunk/varnish-cache/bin/varnishd/stevedore.h 2008-07-19 09:24:07 UTC (rev 2955)
@@ -35,13 +35,15 @@
struct sess;
struct iovec;

-typedef void storage_init_f(struct stevedore *, const char *spec);
+typedef void storage_init_f(struct stevedore *, int ac, char * const *av);
typedef void storage_open_f(const struct stevedore *);
typedef struct storage *storage_alloc_f(struct stevedore *, size_t size);
typedef void storage_trim_f(const struct storage *, size_t size);
typedef void storage_free_f(const struct storage *);

struct stevedore {
+ unsigned magic;
+#define STEVEDORE_MAGIC 0x4baf43db
const char *name;
storage_init_f *init; /* called by mgt process */
storage_open_f *open; /* called by cache process */
@@ -58,5 +60,5 @@
struct storage *STV_alloc(struct sess *sp, size_t size);
void STV_trim(const struct storage *st, size_t size);
void STV_free(const struct storage *st);
-void STV_add(const char *spec);
+void STV_add(const struct stevedore *stv, int ac, char * const *av);
void STV_open(void);

Modified: trunk/varnish-cache/bin/varnishd/storage_file.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/storage_file.c 2008-07-19 07:19:12 UTC (rev 2954)
+++ trunk/varnish-cache/bin/varnishd/storage_file.c 2008-07-19 09:24:07 UTC (rev 2955)
@@ -108,7 +108,7 @@
};

struct smf_sc {
- char *filename;
+ const char *filename;
int fd;
unsigned pagesize;
uintmax_t filesize;
@@ -172,11 +172,8 @@
} else {
q = str2bytes(size, &l, fssize);

- if (q != NULL) {
- fprintf(stderr,
- "Error: (-sfile) size \"%s\": %s\n", size, q);
- exit (2);
- }
+ if (q != NULL)
+ ARGV_ERR("(-sfile) size \"%s\": %s\n", size, q);
}

/*
@@ -206,12 +203,9 @@
/* round down to multiple of filesystem blocksize or pagesize */
l -= (l % bs);

- if (l < MINPAGES * (uintmax_t)sc->pagesize) {
- fprintf(stderr,
- "Error: size too small, at least %ju needed\n",
+ if (l < MINPAGES * (uintmax_t)sc->pagesize)
+ ARGV_ERR("size too small, at least %ju needed\n",
(uintmax_t)MINPAGES * sc->pagesize);
- exit (2);
- }

if (sizeof(void *) == 4 && l > INT32_MAX) { /*lint !e506 !e774 */
fprintf(stderr,
@@ -238,15 +232,33 @@
/* XXX: force block allocation here or in open ? */
}

+static char default_size[] = "50%";
+static char default_filename[] = ".";
+
static void
-smf_init(struct stevedore *parent, const char *spec)
+smf_init(struct stevedore *parent, int ac, char * const *av)
{
- char *size;
- char *p, *q;
+ const char *size, *fn;
+ char *q, *p;
struct stat st;
struct smf_sc *sc;
unsigned u;

+ AZ(av[ac]);
+
+ fn = default_filename;
+ size = default_size;
+
+ if (ac > 2)
+ ARGV_ERR("(-sfile) too many arguments\n");
+ if (ac > 0 && *av[0] != '\0')
+ fn = av[0];
+ if (ac > 1 && *av[1] != '\0')
+ size = av[1];
+
+ AN(fn);
+ AN(size);
+
sc = calloc(sizeof *sc, 1);
XXXAN(sc);
VTAILQ_INIT(&sc->order);
@@ -257,82 +269,52 @@

parent->priv = sc;

- /* If no size specified, use 50% of filesystem free space */
- if (spec == NULL || *spec == '\0')
- asprintf(&p, ".,50%%");
- else if (strchr(spec, ',') == NULL)
- asprintf(&p, "%s,", spec);
- else
- p = strdup(spec);
- XXXAN(p);
- size = strchr(p, ',');
- XXXAN(size);
-
- *size++ = '\0';
-
/* try to create a new file of this name */
- sc->fd = open(p, O_RDWR | O_CREAT | O_EXCL, 0600);
+ sc->fd = open(fn, O_RDWR | O_CREAT | O_EXCL, 0600);
if (sc->fd >= 0) {
- sc->filename = p;
+ sc->filename = fn;
mgt_child_inherit(sc->fd, "storage_file");
smf_initfile(sc, size, 1);
return;
}

/* it must exist then */
- if (stat(p, &st)) {
- fprintf(stderr,
- "Error: (-sfile) \"%s\" "
- "does not exist and could not be created\n", p);
- exit (2);
- }
+ if (stat(fn, &st))
+ ARGV_ERR("(-sfile) \"%s\" "
+ "does not exist and could not be created\n", fn);

/* and it should be a file or directory */
- if (!(S_ISREG(st.st_mode) || S_ISDIR(st.st_mode))) {
- fprintf(stderr,
- "Error: (-sfile) \"%s\" "
- "is neither file nor directory\n", p);
- exit (2);
- }
+ if (!(S_ISREG(st.st_mode) || S_ISDIR(st.st_mode)))
+ ARGV_ERR("(-sfile) \"%s\" is neither file nor directory\n", fn);

if (S_ISREG(st.st_mode)) {
- sc->fd = open(p, O_RDWR);
- if (sc->fd < 0) {
- fprintf(stderr,
- "Error: (-sfile) \"%s\" "
- "could not open (%s)\n", p, strerror(errno));
- exit (2);
- }
+ sc->fd = open(fn, O_RDWR);
+ if (sc->fd < 0)
+ ARGV_ERR("(-sfile) \"%s\" could not open (%s)\n",
+ fn, strerror(errno));
AZ(fstat(sc->fd, &st));
- if (!S_ISREG(st.st_mode)) {
- fprintf(stderr,
- "Error: (-sfile) \"%s\" "
- "was not a file after opening\n", p);
- exit (2);
- }
- sc->filename = p;
+ if (!S_ISREG(st.st_mode))
+ ARGV_ERR("(-sfile) \"%s\" "
+ "was not a file after opening\n", fn);
+ sc->filename = fn;
mgt_child_inherit(sc->fd, "storage_file");
smf_initfile(sc, size, 0);
return;
}

-
- asprintf(&q, "%s/varnish.XXXXXX", p);
+ asprintf(&q, "%s/varnish.XXXXXX", fn);
XXXAN(q);
sc->fd = mkstemp(q);
- if (sc->fd < 0) {
- fprintf(stderr,
- "Error: (-sfile) \"%s\" "
- "mkstemp(%s) failed (%s)\n", p, q, strerror(errno));
- exit (2);
- }
+ if (sc->fd < 0)
+ ARGV_ERR("(-sfile) \"%s\" "
+ "mkstemp(%s) failed (%s)\n", fn, q, strerror(errno));
AZ(unlink(q));
- asprintf(&sc->filename, "%s (unlinked)", q);
- XXXAN(sc->filename);
+ asprintf(&p, "%s (unlinked)", q);
+ XXXAN(p);
+ sc->filename = p;
free(q);
smf_initfile(sc, size, 1);
mgt_child_inherit(sc->fd, "storage_file");
- free(p);
}

/*--------------------------------------------------------------------
@@ -705,12 +687,13 @@
/*--------------------------------------------------------------------*/

struct stevedore smf_stevedore = {
- .name = "file",
- .init = smf_init,
- .open = smf_open,
- .alloc = smf_alloc,
- .trim = smf_trim,
- .free = smf_free,
+ .magic = STEVEDORE_MAGIC,
+ .name = "file",
+ .init = smf_init,
+ .open = smf_open,
+ .alloc = smf_alloc,
+ .trim = smf_trim,
+ .free = smf_free,
};

#ifdef INCLUDE_TEST_DRIVER

Modified: trunk/varnish-cache/bin/varnishd/storage_malloc.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/storage_malloc.c 2008-07-19 07:19:12 UTC (rev 2954)
+++ trunk/varnish-cache/bin/varnishd/storage_malloc.c 2008-07-19 09:24:07 UTC (rev 2955)
@@ -123,26 +123,26 @@
}

static void
-sma_init(struct stevedore *parent, const char *spec)
+sma_init(struct stevedore *parent, int ac, char * const *av)
{
const char *e;
uintmax_t u;

(void)parent;
- if (spec != NULL && *spec != '\0') {
- e = str2bytes(spec, &u, 0);
- if (e != NULL) {
- fprintf(stderr,
- "Error: (-smalloc) size \"%s\": %s\n", spec, e);
- exit(2);
- }
- if ((u != (uintmax_t)(size_t)u)) {
- fprintf(stderr,
- "Error: (-smalloc) size \"%s\": too big\n", spec);
- exit(2);
- }
- sma_max = u;
- }
+
+ AZ(av[ac]);
+ if (ac > 1)
+ ARGV_ERR("(-smalloc) too many arguments\n");
+
+ if (ac == 0 || *av[0] == '\0')
+ return;
+
+ e = str2bytes(av[0], &u, 0);
+ if (e != NULL)
+ ARGV_ERR("(-smalloc) size \"%s\": %s\n", av[0], e);
+ if ((u != (uintmax_t)(size_t)u))
+ ARGV_ERR("(-smalloc) size \"%s\": too big\n", av[0]);
+ sma_max = u;
}

static void
@@ -153,10 +153,11 @@
}

struct stevedore sma_stevedore = {
- .name = "malloc",
- .init = sma_init,
- .open = sma_open,
- .alloc = sma_alloc,
- .free = sma_free,
- .trim = sma_trim,
+ .magic = STEVEDORE_MAGIC,
+ .name = "malloc",
+ .init = sma_init,
+ .open = sma_open,
+ .alloc = sma_alloc,
+ .free = sma_free,
+ .trim = sma_trim,
};

Modified: trunk/varnish-cache/bin/varnishd/varnishd.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/varnishd.c 2008-07-19 07:19:12 UTC (rev 2954)
+++ trunk/varnish-cache/bin/varnishd/varnishd.c 2008-07-19 09:24:07 UTC (rev 2955)
@@ -67,6 +67,7 @@
#include "shmlog.h"
#include "heritage.h"
#include "mgt.h"
+#include "hash_slinger.h"
#include "stevedore.h"

/* INFTIM indicates an infinite timeout for poll(2) */
@@ -79,47 +80,97 @@

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

-static int
-cmp_hash(const struct hash_slinger *s, const char *p, const char *q)
+struct choice {
+ const char *name;
+ void *ptr;
+};
+
+static void *
+pick(const struct choice *cp, const char *which, const char *kind)
{
- if (strlen(s->name) != (q - p))
- return (1);
- if (strncmp(s->name, p, (q - p)))
- return (1);
- return (0);
+
+ for(; cp->name != NULL; cp++) {
+ if (!strcmp(cp->name, which))
+ return (cp->ptr);
+ }
+ ARGV_ERR("Unknown %s method \"%s\"\n", kind, which);
}

+/*--------------------------------------------------------------------*/
+
+extern struct stevedore sma_stevedore;
+extern struct stevedore smf_stevedore;
+
+static struct choice stv_choice[] = {
+ { "file", &smf_stevedore },
+ { "malloc", &sma_stevedore },
+ { NULL, NULL }
+};
+
static void
-setup_hash(const char *s_arg)
+setup_storage(const char *spec)
{
- const char *p, *q;
+ char **av;
+ void *priv;
+ int ac;
+
+ av = ParseArgv(spec, ARGV_COMMA);
+
+ if (av[0] != NULL)
+ ARGV_ERR("%s\n", av[0]);
+
+ if (av[1] == NULL)
+ ARGV_ERR("-s argument is empty\n");
+
+ for (ac = 0; av[ac + 2] != NULL; ac++)
+ continue;
+
+ priv = pick(stv_choice, av[1], "storage");
+ AN(priv);
+
+ STV_add(priv, ac, av + 2);
+
+ /* We do not free av, to make life simpler for stevedores */
+}
+
+/*--------------------------------------------------------------------*/
+
+extern struct hash_slinger hsl_slinger;
+extern struct hash_slinger hcl_slinger;
+
+static struct choice hsh_choice[] = {
+ { "classic", &hcl_slinger },
+ { "simple", &hsl_slinger },
+ { "simple_list", &hsl_slinger }, /* backwards compat */
+ { NULL, NULL }
+};
+
+static void
+setup_hash(const char *h_arg)
+{
+ char **av;
+ int ac;
struct hash_slinger *hp;

- p = strchr(s_arg, ',');
- if (p == NULL)
- q = p = strchr(s_arg, '\0');
- else
- q = p + 1;
- xxxassert(p != NULL);
- xxxassert(q != NULL);
- if (!cmp_hash(&hcl_slinger, s_arg, p)) {
- hp = &hcl_slinger;
- } else if (!cmp_hash(&hsl_slinger, s_arg, p)) {
- hp = &hsl_slinger;
- } else {
- fprintf(stderr, "Unknown hash method \"%.*s\"\n",
- (int)(p - s_arg), s_arg);
- exit (2);
- }
+ av = ParseArgv(h_arg, ARGV_COMMA);
+
+ if (av[0] != NULL)
+ ARGV_ERR("%s\n", av[0]);
+
+ if (av[1] == NULL)
+ ARGV_ERR("-h argument is empty\n");
+
+ for (ac = 0; av[ac + 2] != NULL; ac++)
+ continue;
+
+ hp = pick(hsh_choice, av[1], "hash");
+ CHECK_OBJ_NOTNULL(hp, SLINGER_MAGIC);
heritage.hash = hp;
- if (hp->init != NULL) {
- if (hp->init(q))
- exit (1);
- } else if (*q) {
- fprintf(stderr, "Hash method \"%s\" takes no arguments\n",
+ if (hp->init != NULL)
+ hp->init(ac, av + 2);
+ else if (ac > 0)
+ ARGV_ERR("Hash method \"%s\" takes no arguments\n",
hp->name);
- exit (1);
- }
}

/*--------------------------------------------------------------------*/
@@ -436,7 +487,7 @@
break;
case 's':
s_arg_given = 1;
- STV_add(optarg);
+ setup_storage(optarg);
break;
case 't':
MCF_ParamSet(cli, "default_ttl", optarg);
@@ -538,7 +589,7 @@
exit (0);

if (!s_arg_given)
- STV_add(s_arg);
+ setup_storage(s_arg);

setup_hash(h_arg);