[PATCH 1 of 3] chg: pass sensitive command line flags to server
Yuya Nishihara
yuya at tcha.org
Tue Feb 16 11:10:05 EST 2016
On Tue, 16 Feb 2016 11:12:00 +0000, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark at fb.com>
> # Date 1455618922 0
> # Tue Feb 16 10:35:22 2016 +0000
> # Node ID 3fc701a86323ae896371ec3df375a52bce06b748
> # Parent 923500ca4f9a294db25318db07239359ad131348
> chg: pass sensitive command line flags to server
A couple of comments follow, but they are mostly nitpicking.
> +static void freecmdserveropts(struct cmdserveropts *opts) {
> + if (opts->args) {
> + free(opts->args);
> + opts->args = NULL;
> + opts->argsize = 0;
> + }
free(NULL) is valid, and chg does it at several places.
> +static int testsensetiveflag(const char *arg)
> +{
> + static struct {
> + const char *name;
> + const int narg;
Perhaps you meant
static const struct {
const char *name;
int narg;
}
> + size_t i;
> + for (i = 0; i < sizeof(flags) / sizeof(flags[0]); ++i) {
> + int len = strlen(flags[i].name);
"len" can be size_t.
> + int narg = flags[i].narg;
> + if (!memcmp(arg, flags[i].name, len)) {
I tend to write it as memcmp() == 0 because the return value is not boolean.
But I'm fine with that if you prefer !memcmp().
> +static void *reallocx(void *ptr, size_t size)
> +{
> + void *result = realloc(ptr, size);
> + if (!result) abortmsg("failed to realloc");
> + return result;
> +}
Can it be moved util.c?
hgclient.c has the same pattern.
> +static void setcmdserverargs(struct cmdserveropts *opts,
> + int argc, const char *argv[])
> +{
> + int i, argsize = 0;
> + const char **args = NULL;
> + for (i = 0; i < argc; ++i) {
> + if (!argv[i]) continue; /* pass clang-analyse */
> + if (!strcmp(argv[i], "--")) break;
> + int n = testsensetiveflag(argv[i]);
> + if (n == 0 || i + n > argc) continue;
> + args = reallocx(args, (n + argsize) * sizeof(char*));
> + memcpy(args + argsize, argv + i, sizeof(char*) * n);
> + argsize += n;
> + i += n - 1;
Hmm, isn't it clearer to use "while" than "for" ?
while (i < argc) {
...
i += n;
}
How can we handle "ci -m --config foo" ?
Sorry, I didn't notice it before.
> + }
> + if (opts->args) free(opts->args);
Perhaps, we can realloc() old opts->args.
More information about the Mercurial-devel
mailing list