git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Rafael Silva <rafaeloliveira.cs@gmail.com>
Date
Dec 23, 2020, 00:23 UTC
Message-ID
<gohp6klfdpbca4.fsf@cpm12071.fritz.box>
In-Reply-To
<20201221162743.96056-3-mirucam@gmail.com>
Hi Miriam,
Miriam Rubio writes:
Show 13 quoted lines
> +
> +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
Previous: Rafael SilvaNext: Junio C Hamano
Message 12 of 18 in “Finish converting git bisect to C part 3”
  1. 0/7 Finish converting git bisect to C part 3Miriam Rubio, Dec 21, 2020
  2. 5/7 bisect--helper: retire `--bisect-auto-next` subcommandMiriam Rubio, Dec 21, 2020
  3. 6/7 bisect--helper: reimplement `bisect_skip` shell function in CMiriam Rubio, Dec 21, 2020
  4. Johannes SchindelinJan 18, 2021
  5. 1/7 bisect--helper: reimplement `bisect_log` shell function in CMiriam Rubio, Dec 21, 2020
  6. Johannes SchindelinJan 18, 2021
  7. 3/7 bisect--helper: retire `--bisect-write` subcommandMiriam Rubio, Dec 21, 2020
  8. 7/7 bisect--helper: retire `--check-and-set-terms` subcommandMiriam Rubio, Dec 21, 2020
  9. 4/7 bisect--helper: use `res` instead of return in BISECT_RESET case optionMiriam Rubio, Dec 21, 2020
  10. 2/7 bisect--helper: reimplement `bisect_replay` shell function in CMiriam Rubio, Dec 21, 2020
  11. Rafael SilvaDec 22, 2020
  12. Rafael SilvaDec 23, 2020
  13. Junio C HamanoDec 23, 2020
  14. Rafael SilvaDec 23, 2020
  15. Junio C HamanoDec 23, 2020
  16. Johannes SchindelinJan 18, 2021
  17. Johannes SchindelinJan 18, 2021
  18. Junio C HamanoJan 18, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.