2006/9/8, Rene Scharfe <rene.scharfe@xxxxxxxxxxxxxx>:
Only a few trivial comments, as I managed to catch a cold somehow and
can't think straight for longer than three seconds.
> .gitignore | 1
> Documentation/git-archive.txt | 100 ++++++++++++++++++
> Makefile | 3 -
[snip]
> +
> + url = strdup(ar->remote);
xstrdup()
ok, but need to rebase...
> + pid = git_connect(fd, url, buf);
> + if (pid < 0)
> + return pid;
> +
[snip]
> + int extra_argc = 0;
> + const char *format = NULL; /* some default values */
This comment does not convey any information.
OK, I'll remove it
> + const char *remote = NULL;
> + const char *base = "";
[snip]
> + }
> + if (arg[0] == '-') {
> + extra_argv[extra_argc++] = arg;
Overrun is not checked.
Indeed, I'll fix it.
> + continue;
> + }
> + break;
> + }
> + if (list) {
> + if (!remote) {
> + for (i = 0; i < ARRAY_SIZE(archivers); i++)
> + printf("%s\n", archivers[i].name);
> + exit(0);
> + }
> + die("--list and --remote are mutually exclusive");
> + }
Not sure if we really need a list option. I guess it only really
makes sense if we have more than five formats. I have no _strong_
feelings against it, though. *shrug*
well it's almost free to add it, and no need any maintenance if we add
a new archiver backend, so I would say let it.
> + if (argc - i < 1) {
> + die("%s", archive_usage);
usage()
ok
--
Franck
-
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