Yann Dirson wrote:
> --- 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()?)
> @@ -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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html