summaryrefslogtreecommitdiff
path: root/combine-diff.c
diff options
context:
space:
mode:
authorJunio C Hamano <gitster@pobox.com>2014-01-28 13:55:59 -0800
committerJunio C Hamano <gitster@pobox.com>2014-02-24 14:44:57 -0800
commit7b1004b0ba6637e8c299ee8f927de5426139495c (patch)
tree6127fed901c8d8d52db795bb4e7fe7b278c6787f /combine-diff.c
parentaf82c7880f1a3df1655092da11c80603260384a0 (diff)
downloadgit-7b1004b0ba6637e8c299ee8f927de5426139495c.tar.gz
combine-diff: simplify intersect_paths() further
Linus once said: I actually wish more people understood the really core low-level kind of coding. Not big, complex stuff like the lockless name lookup, but simply good use of pointers-to-pointers etc. For example, I've seen too many people who delete a singly-linked list entry by keeping track of the "prev" entry, and then to delete the entry, doing something like if (prev) prev->next = entry->next; else list_head = entry->next; and whenever I see code like that, I just go "This person doesn't understand pointers". And it's sadly quite common. People who understand pointers just use a "pointer to the entry pointer", and initialize that with the address of the list_head. And then as they traverse the list, they can remove the entry without using any conditionals, by just doing a "*pp = entry->next". Applying that simplification lets us lose 7 lines from this function even while adding 2 lines of comment. I was tempted to squash this into the original commit, but because the benchmarking described in the commit log is without this simplification, I decided to keep it a separate follow-up patch. Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'combine-diff.c')
-rw-r--r--combine-diff.c34
1 files changed, 12 insertions, 22 deletions
diff --git a/combine-diff.c b/combine-diff.c
index 2d79312a0b..24ca7e2334 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -15,11 +15,10 @@
static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, int n, int num_parent)
{
struct diff_queue_struct *q = &diff_queued_diff;
- struct combine_diff_path *p, *pprev, *ptmp;
+ struct combine_diff_path *p, **tail = &curr;
int i, cmp;
if (!n) {
- struct combine_diff_path *list = NULL, **tail = &list;
for (i = 0; i < q->nr; i++) {
int len;
const char *path;
@@ -43,35 +42,27 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,
*tail = p;
tail = &p->next;
}
- return list;
+ return curr;
}
/*
- * NOTE paths are coming sorted here (= in tree order)
+ * paths in curr (linked list) and q->queue[] (array) are
+ * both sorted in the tree order.
*/
-
- pprev = NULL;
- p = curr;
i = 0;
+ while ((p = *tail) != NULL) {
+ cmp = ((i >= q->nr)
+ ? -1 : strcmp(p->path, q->queue[i]->two->path));
- while (1) {
- if (!p)
- break;
-
- cmp = (i >= q->nr) ? -1
- : strcmp(p->path, q->queue[i]->two->path);
if (cmp < 0) {
- if (pprev)
- pprev->next = p->next;
- ptmp = p;
- p = p->next;
- free(ptmp);
- if (curr == ptmp)
- curr = p;
+ /* p->path not in q->queue[]; drop it */
+ *tail = p->next;
+ free(p);
continue;
}
if (cmp > 0) {
+ /* q->queue[i] not in p->path; skip it */
i++;
continue;
}
@@ -80,8 +71,7 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,
p->parent[n].mode = q->queue[i]->one->mode;
p->parent[n].status = q->queue[i]->status;
- pprev = p;
- p = p->next;
+ tail = &p->next;
i++;
}
return curr;