"Rose via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: Seija Kijin <doremylover123@xxxxxxxxx>
>
> strncmp does not modify any of the memory.
It may be a correct statement, but so what? It does not seem to
have relevance to this change.
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 0f093f2a4f2..74de2ebd2b3 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -188,7 +188,7 @@ static struct strategy *get_strategy(const char *name)
> for (i = 0; i < main_cmds.cnt; i++) {
> int j, found = 0;
> struct cmdname *ent = main_cmds.names[i];
> - for (j = 0; j < ARRAY_SIZE(all_strategy); j++)
> + for (j = 0; !found && j < ARRAY_SIZE(all_strategy); j++)
> if (!strncmp(ent->name, all_strategy[j].name, ent->len)
> && !all_strategy[j].name[ent->len])
> found = 1;
I am not sure if this micro-optimization is worth it.
If this loop is so costly that it needs optimization, a better thing
to do would be to rethink the way it filters main_cmds.names[] array
with all_strategy[]. The latter is a fairly small, and more
importantly, a constant set of known strategies, so there should be
a more efficient way than O(n*m) nested loop.
The code churn has already costed us too much, mostly reviewer and
maintainer time, for the value of the change itself. I'll queue the
patch as-is, because this change is not making anything worse
per-se, but primarily because I do not want the topic to take any
more of our resources.
Thanks.
