Re: [PATCH 1/4] Start to replace locate_rename_dst() with a generic function.
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:49:58
Yann Dirson wrote:
quoted hunk ↗ jump to hunk
--- a/diffcore-rename.c +++ b/diffcore-rename.c@@ -6,6 +6,10 @@ #include "diffcore.h" #include "hash.h" +#define locate_element(list,elem,insert_ok) \ + _locate_element(elem, &list##_nr, &list##_alloc, \ + insert_ok) +
Is this syntactic sugar needed?
static struct diff_rename_dst *locate_rename_dst(struct diff_filespec *two,
insert_ok)
{
return locate_element(&rename_dst_nr, &rename_dst_alloc, elem, insert_ok);
}
takes more advantage of the compiler's typechecking and looks easy
enough to read.
Since this is local to diffcore-rename, I don't mind the locate_element()
name, but if this is to be used more widely I think it would need to be
named more precisely. (find_or_insert_in_array()?)
quoted hunk ↗ jump to hunk
@@ -13,14 +17,17 @@ static struct diff_rename_dst {
[...]
-static struct diff_rename_dst *locate_rename_dst(struct diff_filespec *two,
- int insert_ok)
+static struct diff_rename_dst *_locate_element(struct diff_filespec *two,
+ int *elem_nr_p, int *elem_alloc_p,
+ int insert_ok)
{
int first, last;
first = 0;
- last = rename_dst_nr;
+ last = (*elem_nr_p);I guess these parentheses came from search+replace? It's more readable without them. [...]
+ (*elem_nr_p)++;
Except for this one. Generally, the approach seems sane so far.