I have a few stylistic comments but the contents look quite sane
and fine.
> I use an inetd invocation like this example:
>
> git stream tcp nowait nobody /usr/bin/git-daemon git-daemon --inetd
> --verbose --syslog
> --export-all --interpolated-path=/pub/%H/%D
> /pub/software /software
> /pub/www.example.com/software
> /pub/www.example.org/software
> /pub
This would be nice to have as an example in the documentation.
> @@ -45,6 +51,23 @@ static const char *user_path;
> static unsigned int timeout;
> static unsigned int init_timeout;
>
> +/*
> + * Static table for now. Ugh.
> + * Feel free to make dynamic as needed.
> + */
> +#define INTERP_SLOT_HOST (0)
> +#define INTERP_SLOT_DIR (1)
> +#define INTERP_SLOT_PERCENT (2)
> +
> +struct interp interp_table[] = {
> + { "%H", 0},
> + { "%D", 0},
> + { "%%", "%"},
> +};
> +
> +#define N_INTERPS (sizeof(interp_table) / sizeof(struct interp))
Shouldn't this variable static to the file?
We seem to use ARRAY_SIZE() macro, which is not particularly my
favorite but other places already use it so probably we would
want to keep consistency.
> + loginfo("Before interpolation '%s'", dir);
> + loginfo("Interp slot 0 (%s,%s)",
> + interp_table[0].name, interp_table[0].value);
> + loginfo("Interp slot 1 (%s,%s)",
> + interp_table[1].name, interp_table[1].value);
> + interpolate(interp_path, PATH_MAX, interpolated_path,
> + interp_table, N_INTERPS);
> + loginfo("After interpolation '%s'", interp_path);
> + dir = interp_path;
Do we really want to see each step of interpolation in the log?
Sounds more like logdebug than loginfo to me.
> + snprintf(rpath, PATH_MAX, "%s%s", base_path, dir);
> + loginfo("dir was %s", dir);
> + loginfo("base_path is %s", base_path);
> + loginfo("rpath now %s", rpath);
> + dir = rpath;
Likewise.
> @@ -351,6 +398,29 @@ static void make_service_overridable(con
> die("No such service %s", name);
> }
>
> +void parse_extra_args(char *extra_args, int buflen)
static?
> @@ -391,13 +461,19 @@ #endif
> if (len && line[len-1] == '\n')
> line[--len] = 0;
>
> + if (len != pktlen) {
> + parse_extra_args(line + len + 1, pktlen - len - 1);
> + }
> +
No { } around single statement.
> +int
> +interpolate(char *result, int reslen,
> + char *orig,
> + struct interp *interps, int ninterps)
> +{
"int interpolate(..."; the return type and function name on the
same line (again, not my preference but just doing as Romans do).
> + bzero(result, reslen);
memset(result, 0, reslen);
> diff --git a/interpolate.h b/interpolate.h
> new file mode 100644
> index 0000000..241af7c
> --- /dev/null
> +++ b/interpolate.h
> @@ -0,0 +1,13 @@
#ifndef INTERPOLATE_H
#define INTERPOLATE_H
...
#endif
> +/*
> + * Copyright 2006 Jon Loeliger
> + */
> +
> +struct interp {
> + char *name;
> + char *value;
> +};
> +
> +extern int interpolate(char *result, int reslen,
> + char *orig,
> + struct interp *interps, int ninterps);
> +
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html