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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux