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

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

+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 ...

+		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.

+			}
+		}
+
+		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 ...)

+		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