[PATCH 1 of 3] chg: pass sensitive command line flags to server

[Yuya Nishihara]

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.