Greg Brockman <gdb@xxxxxxx> writes:
> This provides a mechanism for the server to expose custom
> functionality to clients. My particular use case is that I would like
> a way of discovering all repositories available for cloning. A
> client that clones via
>
> git clone user@xxxxxxxxxxx
>
> can invoke a command by
>
> ssh user@xxxxxxxxxxx $command
>
> Signed-off-by: Greg Brockman <gdb@xxxxxxx>
> ---
Just a few minor nits...
> +static char *make_cmd(const char *prog)
> +{
> + char *prefix = xmalloc((strlen(prog) + strlen(COMMAND_DIR) + 2) * sizeof(char));
Since sizeof(char) by defintion is 1, I'd rather not to see the above
obfuscation.
> @@ -48,6 +64,8 @@ static struct commands {
> int main(int argc, char **argv)
> {
> char *prog;
> + char *prog_cpy;
> + const char **user_argv;
> struct commands *cmd;
> int devnull_fd;
>
> @@ -77,6 +95,7 @@ int main(int argc, char **argv)
> die("What do you think I am? A shell?");
>
> prog = argv[2];
> + prog_cpy = xstrdup(prog);
Wouldn't changing "prog = argv[2]" to "prog = xstrdup(argv[2])" easier to
read? Or name this new variable "prog_orig", have one comment here to say
"keep the original for reporting purposes, as prog will be munged", and
drop the two 4-line comment blocks below.
> if (!strncmp(prog, "git", 3) && isspace(prog[3]))
> /* Accept "git foo" as if the caller said "git-foo". */
> prog[3] = '-';
> @@ -99,5 +118,25 @@ int main(int argc, char **argv)
> }
> exit(cmd->exec(cmd->name, arg));
> }
> - die("unrecognized command '%s'", prog);
> +
> + if (split_cmdline(prog, &user_argv) != -1) {
> + if (is_valid_cmd_name(user_argv[0])) {
> + prog = make_cmd(user_argv[0]);
Extra SP before "="?
> + user_argv[0] = prog;
> + execv(user_argv[0], (char *const *) user_argv);
> + free(prog);
> + }
> + free(user_argv);
> + /*
> + * split_cmdline modifies its argument in-place, so 'prog' now
> + * holds the actual command name
> + */
> + die("unrecognized command '%s'", prog_cpy);
> + } else {
> + /*
> + * split_cmdline has clobbered prog and printed an
> + * error message, so print the original
> + */
> + die("invalid command format '%s'", prog_cpy);
Hmm, we might want to fix split_cmdline to report the breakage better
then (perhaps not as part of this patch series). Instead of seeing
error: cmdline ends with \.
fatal: invalid command format 'foo bar \'.
wouldn't it be nicer to see a single error message:
fatal: invalid command 'foo bar\': cmdline ends with \.
--
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