Re: [PATCH] Add virtualization support to git-daemon
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:40
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
/pubThis would be nice to have as an example in the documentation.
quoted hunk
@@ -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.
quoted hunk
@@ -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?
quoted hunk
@@ -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);
quoted hunk
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);
+