@matheusbsbs / matheus-bagliano / commits / 7b1004b0ba

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>

Junio C Hamano committed Jan 28, 2014 at 13:55 UTC 7b1004b0ba6637e8c299ee8f927de5426139495c

1 file changed +12 -22

combine-diff.c

+12 -22

@@ -15,11 +15,10 @@

15 static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, int n, int num_parent)

16 {

17 struct diff_queue_struct *q = &diff_queued_diff;

18 - struct combine_diff_path *p, *pprev, *ptmp;

18 + struct combine_diff_path *p, **tail = &curr;

19 int i, cmp;

21 if (!n) {

22 - struct combine_diff_path *list = NULL, **tail = &list;

22 for (i = 0; i < q->nr; i++) {

23 int len;

24 const char *path;

@@ -43,35 +42,27 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,

42 *tail = p;

43 tail = &p->next;

44 }

46 - return list;

45 + return curr;

46 }

48 /*

50 - * NOTE paths are coming sorted here (= in tree order)

49 + * paths in curr (linked list) and q->queue[] (array) are

50 + * both sorted in the tree order.

51 */

52 -

53 - pprev = NULL;

54 - p = curr;

52 i = 0;

53 + while ((p = *tail) != NULL) {

54 + cmp = ((i >= q->nr)

55 + ? -1 : strcmp(p->path, q->queue[i]->two->path));

57 - while (1) {

58 - if (!p)

59 - break;

60 -

61 - cmp = (i >= q->nr) ? -1

62 - : strcmp(p->path, q->queue[i]->two->path);

57 if (cmp < 0) {

64 - if (pprev)

65 - pprev->next = p->next;

66 - ptmp = p;

67 - p = p->next;

68 - free(ptmp);

69 - if (curr == ptmp)

70 - curr = p;

58 + /* p->path not in q->queue[]; drop it */

59 + *tail = p->next;

60 + free(p);

61 continue;

62 }

64 if (cmp > 0) {

65 + /* q->queue[i] not in p->path; skip it */

66 i++;

67 continue;

68 }

@@ -80,8 +71,7 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,

71 p->parent[n].mode = q->queue[i]->one->mode;

72 p->parent[n].status = q->queue[i]->status;

83 - pprev = p;

84 - p = p->next;

74 + tail = &p->next;

75 i++;

76 }

77 return curr;