Hi,
On Sat, 15 Dec 2007, Steven Grimm wrote:
> @@ -238,7 +186,7 @@ struct cmd_struct {
> int option;
> };
>
> -static int run_command(struct cmd_struct *p, int argc, const char **argv)
> +static int run_git_command(struct cmd_struct *p, int argc, const char **argv)
> {
> int status;
> struct stat st;
Funny ;-) I have a similar change in my (now-inactive until 1.5.4) tree,
but I renamed it to execv_git_builtin (because it is not supposed to
return in case of success).
> diff --git a/run-command.c b/run-command.c
> index 476d00c..3ae55ec 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -237,3 +237,68 @@ int finish_async(struct async *async)
> ret = error("waitpid (async) failed");
> return ret;
> }
> +
> +/*
> + * Parses a command line into an array of char* representing the tokens
> + * on the command line. Pass in a count to reserve some number of additional
> + * slots in the allocated array, e.g., so the caller can add a filename
> + * argument without having to reallocate the array.
In the editor call, you said "2" for this, but I suspect that you counted
the NULL extra. However, in git.c you said "0", thus not counting the
NULL. IMHO it makes sense _not_ to count the NULL (and NULL-terminate
the list _always_).
> +int split_cmdline(char *cmdline, const char ***argv, int extra_slots)
> +{
> + int src, dst, count = 0, size = extra_slots + 16;
> + char quoted = 0;
> +
> + *argv = xmalloc(sizeof(char*) * size);
> +
> + /* split alias_string */
s/alias_string/the command line/
> + (*argv)[count++] = cmdline;
> + for (src = dst = 0; cmdline[src];) {
> + char c = cmdline[src];
> + if (!quoted && isspace(c)) {
> + cmdline[dst++] = 0;
> + while (cmdline[++src]
> + && isspace(cmdline[src]))
> + ; /* skip */
> + if (count >= size) {
s/count/count + extra_slots/
> + size += 16;
> + *argv = xrealloc(*argv, sizeof(char*) * size);
This seems a nice candidate for ALLOC_GROW() in any case:
ALLOC_GROW(*argv, count + extra_slots + 1, size);
> + }
> + (*argv)[count++] = cmdline + dst;
> + } else if(!quoted && (c == '\'' || c == '"')) {
> + quoted = c;
> + src++;
> + } else if (c == quoted) {
> + quoted = 0;
> + src++;
> + } else {
> + if (c == '\\' && quoted != '\'') {
> + src++;
> + c = cmdline[src];
> + if (!c) {
> + free(*argv);
> + *argv = NULL;
> + return error("cmdline ends with \\");
> + }
> + }
> + cmdline[dst++] = c;
> + src++;
> + }
> + }
> +
> + cmdline[dst] = 0;
> +
> + if (quoted) {
> + free(*argv);
> + *argv = NULL;
> + return error("unclosed quote");
> + }
AFAICT the argv were not NULL terminated before. But now that the
function is public, it is safer to do so:
(*argv)[count] = NULL;
> +
> + return count;
> +}
> +
Thanks,
Dscho
-
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
