Merge pull request #2191 from dscho/add-p-fixups

A couple of fixups for add-i-in-c-status-and-help

Author: dscho

add-interactive.c

@@ -138,9 +138,15 @@ struct list_and_choose_options {
 	void (*print_help)(struct add_i_state *s);
 };
 
+#define LIST_AND_CHOOSE_ERROR (-1)
+#define LIST_AND_CHOOSE_QUIT  (-2)
+
 /*
  * Returns the selected index in singleton mode, the number of selected items
  * otherwise.
+ *
+ * If an error occurred, returns `LIST_AND_CHOOSE_ERROR`. Upon EOF,
+ * `LIST_AND_CHOOSE_QUIT` is returned.
  */
 static ssize_t list_and_choose(struct prefix_item **items, int *selected,
 			       size_t nr, struct add_i_state *s,
@@ -150,7 +156,7 @@ static ssize_t list_and_choose(struct prefix_item **items, int *selected,
 	int immediate = opts->flags & IMMEDIATE;
 
 	struct strbuf input = STRBUF_INIT;
-	ssize_t res = singleton ? -1 : 0;
+	ssize_t res = singleton ? LIST_AND_CHOOSE_ERROR : 0;
 
 	if (!selected && !singleton)
 		BUG("need a selected array in non-singleton mode");
@@ -174,7 +180,7 @@ static ssize_t list_and_choose(struct prefix_item **items, int *selected,
 		if (strbuf_getline(&input, stdin) == EOF) {
 			putchar('\n');
 			if (immediate)
-				res = -2;
+				res = LIST_AND_CHOOSE_QUIT;
 			break;
 		}
 		strbuf_trim(&input);
@@ -253,7 +259,8 @@ static ssize_t list_and_choose(struct prefix_item **items, int *selected,
 			p += sep + 1;
 		}
 
-		if ((immediate && res >= 0) || !strcmp(input.buf, "*"))
+		if ((immediate && res != LIST_AND_CHOOSE_ERROR) ||
+		    !strcmp(input.buf, "*"))
 			break;
 	}
 
@@ -1050,12 +1057,13 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
 	for (;;) {
 		i = list_and_choose((struct prefix_item **)commands, NULL,
 				    ARRAY_SIZE(commands), &s, &main_loop_opts);
-		if (i < -1 || (i >= 0 && !commands[i]->command)) {
+		if (i == LIST_AND_CHOOSE_QUIT ||
+		    (i != LIST_AND_CHOOSE_ERROR && !commands[i]->command)) {
 			printf(_("Bye.\n"));
 			res = 0;
 			break;
 		}
-		if (i >= 0)
+		if (i != LIST_AND_CHOOSE_ERROR)
 			res = commands[i]->command(&s, ps, &files, &opts);
 	}
 

prefix-map.c

@@ -35,40 +35,40 @@ static void init_prefix_map(struct prefix_map *prefix_map,
 static void add_prefix_item(struct prefix_map *prefix_map,
 			    struct prefix_item *item)
 {
-	struct prefix_map_entry *e = xmalloc(sizeof(*e)), *e2;
+	struct prefix_map_entry e = { { NULL } }, *e2;
 	int j;
 
-	e->item = item;
-	e->name = e->item->name;
+	e.item = item;
+	e.name = item->name;
 
-	for (j = prefix_map->min_length; j <= prefix_map->max_length; j++) {
-		if (!isascii(e->name[j])) {
-			free(e);
+	for (j = prefix_map->min_length;
+	     j <= prefix_map->max_length && e.name[j]; j++) {
+		/* Avoid breaking UTF-8 multi-byte sequences */
+		if (!isascii(e.name[j]))
 			break;
-		}
 
-		e->prefix_length = j;
-		hashmap_entry_init(e, memhash(e->name, j));
-		e2 = hashmap_get(&prefix_map->map, e, NULL);
+		e.prefix_length = j;
+		hashmap_entry_init(&e, memhash(e.name, j));
+		e2 = hashmap_get(&prefix_map->map, &e, NULL);
 		if (!e2) {
-			/* prefix is unique so far */
-			e->item->prefix_length = j;
-			hashmap_add(&prefix_map->map, e);
+			/* prefix is unique at this stage */
+			item->prefix_length = j;
+			add_prefix_entry(&prefix_map->map, e.name, j, item);
 			break;
 		}
 
 		if (!e2->item)
 			continue; /* non-unique prefix */
 
-		if (j != e2->item->prefix_length)
-			BUG("unexpected prefix length: %d != %d",
-			    (int)j, (int)e2->item->prefix_length);
+		if (j != e2->item->prefix_length || memcmp(e.name, e2->name, j))
+			BUG("unexpected prefix length: %d != %d (%s != %s)",
+			    j, (int)e2->item->prefix_length, e.name, e2->name);
 
 		/* skip common prefix */
-		for (; j < prefix_map->max_length && e->name[j]; j++) {
-			if (e->item->name[j] != e2->item->name[j])
+		for (; j < prefix_map->max_length && e.name[j]; j++) {
+			if (e.item->name[j] != e2->item->name[j])
 				break;
-			add_prefix_entry(&prefix_map->map, e->name, j + 1,
+			add_prefix_entry(&prefix_map->map, e.name, j + 1,
 					 NULL);
 		}
 
@@ -83,16 +83,14 @@ static void add_prefix_item(struct prefix_map *prefix_map,
 			e2->item->prefix_length = 0;
 		e2->item = NULL;
 
-		if (j < prefix_map->max_length && e->name[j]) {
+		if (j < prefix_map->max_length && e.name[j]) {
 			/* found a unique prefix for the item */
-			e->item->prefix_length = j + 1;
-			add_prefix_entry(&prefix_map->map, e->name, j + 1,
-					 e->item);
-		} else {
+			e.item->prefix_length = j + 1;
+			add_prefix_entry(&prefix_map->map, e.name, j + 1,
+					 e.item);
+		} else
 			/* item has no (short enough) unique prefix */
-			e->item->prefix_length = 0;
-			free(e);
-		}
+			e.item->prefix_length = 0;
 
 		break;
 	}