Re: [PATCH v2 2/7] bisect--helper: reimplement `bisect_replay` shell function in C

[Rafael Silva <rafaeloliveira.cs@xxxxxxxxx>]

Hi Miriam,

Miriam Rubio writes:

> +
> +static int process_replay_file(FILE *fp, struct bisect_terms *terms)
> +{
> +	struct strbuf line = STRBUF_INIT;
> +	struct strbuf word = STRBUF_INIT;
> +	int res = 0;
> +
> +	while (strbuf_getline(&line, fp) != EOF) {
> +		res = process_line(terms, &line, &word);
> +		if (res)
> +			break;
> +	}
> +

I spotted another place where an optimization can be performed.

The "if (res) .. break" conditional is only used to break the loop,
which is the same job of the expression from the while-loop
itself. Hence, as the purpose is to control the loop execution itself,
checking the response of "process_line()" via the "res" value can be
move to the loop expression itself and simplifying the code further,
as shown on the following patch:

-- >8 --

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index b887413d8d..fb15587af8 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -988,11 +988,8 @@ static int process_replay_file(FILE *fp, struct bisect_terms *terms)
        struct strbuf word = STRBUF_INIT;
        int res = 0;

-       while (strbuf_getline(&line, fp) != EOF) {
+       while (!res && strbuf_getline(&line, fp) != EOF)
                res = process_line(terms, &line, &word);
-               if (res)
-                       break;
-       }

        strbuf_release(&line);
        strbuf_release(&word);

-- >8 --

This also seems to be similar approach from "one_of()" introduced by
4ba1e5c414 (bisect--helper: rewrite `check_term_format` shell function
in C, 2017-09-29).

Once more, the intention is to simplify the code and improve the code
maintainability.

> +	strbuf_release(&line);
> +	strbuf_release(&word);
> +	return res;
> +}
> +

I hope this helps increase the code quality.
Happy Holidays

-- 
Thanks
Rafael