diff options
author | Junio C Hamano <gitster@pobox.com> | 2014-01-28 13:55:59 -0800 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2014-02-24 14:44:57 -0800 |
commit | 7b1004b0ba6637e8c299ee8f927de5426139495c (patch) | |
tree | 6127fed901c8d8d52db795bb4e7fe7b278c6787f /combine-diff.c | |
parent | af82c7880f1a3df1655092da11c80603260384a0 (diff) | |
download | git-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.c | 34 |
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; |