Re: [PATCH v6 02/11] Add git-column and column mode parsing

From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date: Feb 27, 2012, 20:09 UTC
Message-ID: <4F4BE300.2060302@ramsay1.demon.co.uk>
In-Reply-To: <1330170078-29353-3-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy wrote: [...]

Show 8 quoted lines

> +static int parse_option(const char *arg, int len,
> +			unsigned int *mode, int stdout_is_tty)
> +{
> +	struct colopt opts[] = {
> +		{ ENABLE, "always",  1 },
> +		{ ENABLE, "never",   0 },
> +		{ ENABLE, "auto",   -1 },
> +	};

Hmm, I don't recognise this table from last time ...

> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(opts); i++) {
> +		int set = 1, arg_len = len, name_len;

set is initialised here (mainly to silence the compiler) in each loop, but then ...

Show 9 quoted lines

> +		const char *arg_str = arg;
> +
> +		if (opts[i].type == OPTION) {
> +			if (arg_len > 2 && !strncmp(arg_str, "no", 2)) {
> +				arg_str += 2;
> +				arg_len -= 2;
> +				set = 0;
> +			} else {
> +				set = 1;

... this else clause is no longer required.

Show 12 quoted lines

> +			}
> +		}
> +
> +		name_len = strlen(opts[i].name);
> +		if (arg_len != name_len ||
> +		    strncmp(arg_str, opts[i].name, name_len))
> +			continue;
> +
> +		switch (opts[i].type) {
> +		case ENABLE:
> +			return set_enable_bit(mode, opts[i].value,
> +					      stdout_is_tty);

given the above table, can the following case limbs ever be reached? (the "no" prefix is only applied to the OPTION type, so most of the above code seems to be useless now ...)

Show 12 quoted lines

> +		case MODE:
> +			return set_mode(mode, opts[i].value);
> +		case OPTION:
> +			return set_option(mode, opts[i].value, set);
> +		default:
> +			die("BUG: Unknown option type %d", opts[i].type);
> +		}
> +	}
> +
> +	return error("unsupported style '%s'", arg);
> +}
> +

[...]

Note, I only skimmed the patch text, I haven't applied it or tested it ...

ATB, Ramsay Jones