Re: [PATCH 04/14] connect: rewrite feature parsing to work on string_list

On Fri, 2016-04-29 at 16:34 -0700, Stefan Beller wrote:

+		if (skip_prefix(item->string, "symref", &val)) {
+			if (!val)
+				continue;

This if should never happen (skip_prefix returns 0 in that case).  You
probably meant !*val -- but:

+			val++; /* skip the = */

I think you should instead skip_prefix "symref=" because:
(a) it saves some code.
(b) it allows for capabilities like symref_foo to later be added.

+	struct string_list list = STRING_LIST_INIT_NODUP;

Maybe move the scope of list into the while loop below?

 	char *line = packet_read_line(0, NULL);
 	while (line) {
-		parse_features(line);
+		string_list_append(&list, line);
+		parse_features(&list);
+		string_list_clear(&list, 1);
 		line = packet_read_line(0, NULL);

This is a bit convoluted in the one-feature-per-line case, but I guess
I understand that for the sake of generality it's useful.