Re: [PATCH] refactoring: yes: Remove unused and complex condition

[Alireza Arzehgar]

Related patch:

From: alireza <alirezaarzehgar82@gmail.com>
Date: Sun, 20 Nov 2022 18:14:49 +0330
Subject: [PATCH 2/2] refactoring: yes: Change mechanism to allocate memory
to
 `buf`

When `bufalloc <= BUFSIZ / 2` is true, code assign `BUFSIZ` to bufalloc.
This action is not required. When using `xmalloc` everytimes it's value is
equal to
`BUFSIZ`.
Using `reuse_operand_strings` can add more code in source file and binary
file.
Replacing this variable with a condition will make code more readable.

Signed-off-by: alireza <alirezaarzehgar82@gmail.com>
---
 src/yes.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/src/yes.c b/src/yes.c
index 7dbf67c19..0916aaa30 100644
--- a/src/yes.c
+++ b/src/yes.c
@@ -82,23 +82,16 @@ main (int argc, char **argv)
   while (++operandp < operand_lim)
       bufalloc += strlen (*operandp) + 1;

-  /* Improve performance by using a buffer size greater than BUFSIZ / 2.
 */
-  bool reuse_operand_strings = true;
-  if (bufalloc <= BUFSIZ / 2)
-    {
-      bufalloc = BUFSIZ;
-      reuse_operand_strings = false;
-    }
-
-  /* Fill the buffer with one copy of the output.  If possible, reuse
-     the operands strings; this wins when the buffer would be large.  */
-  char *buf = reuse_operand_strings ? *operands : xmalloc (bufalloc);
+  /* Improve performance by using a buffer size greater than BUFSIZ / 2.
+     Fill the buffer with one copy of the output.  If possible, reuse
+     the operands strings; this wins when the buffer would be large. */
+  char *buf = bufalloc > BUFSIZ / 2 ? *operands : xmalloc (BUFSIZ);
   size_t bufused = 0;
   operandp = operands;
   do
     {
       size_t operand_len = strlen (*operandp);
-      if (! reuse_operand_strings)
+      if (bufalloc <= BUFSIZ / 2)
         memcpy (buf + bufused, *operandp, operand_len);
       bufused += operand_len;
       buf[bufused++] = ' ';
-- 
2.30.2



On Sun, Nov 20, 2022 at 5:28 PM Alireza Arzehgar <
alirezaarzehgar82@gmail.com> wrote:

> I thought for hours to find usage of this code, but unfortunately I could
> not. Then prepare this patch. I hope this patch was useful.
> We can refactor this code and make simpler source code. I explained my
> reasons to remove this conditions on commit message. Reducing complexity
> and size of code will increase code quality.
> Anyway, removing this block of code will not change on test result. I
> think one of these options should wrong, test or code. If testing of this
> code was ignored, then after understanding what this code does, I should
> add more tests on `yes.sh`.
>
> From: alireza <alirezaarzehgar82@gmail.com>
> Date: Sun, 20 Nov 2022 17:02:24 +0330
> Subject: [PATCH] refactoring: yes: Remove unused and complex condition
>
> Code never pass following conditions:
>
> ```
> if (operandp + 1 < operand_lim
>           && *operandp + operand_len + 1 != operandp[1])
> ```
>
>  - This conditions is true when before `operand_lim`, `operandp` lenght is
> smaller
> than actual memory size and a gap is in the string that `operandp` points
> to.
>  - Enabling `reuse_operand_strings` doesn't prevent any errors or problems
> cause
> problem on counting lenght of string in this block code is similar to
> other parts
> of `yes.c`.
>  - Without this code, tests will successfully passed.
>  - This part of code is very complex. Removing this code will increase
> simplisity
> on `yes.c`.
>
> Signed-off-by: alireza <alirezaarzehgar82@gmail.com>
> ---
>  src/yes.c | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/src/yes.c b/src/yes.c
> index 13b990e24..418cbf36f 100644
> --- a/src/yes.c
> +++ b/src/yes.c
> @@ -78,19 +78,12 @@ main (int argc, char **argv)
>    /* Buffer data locally once, rather than having the
>       large overhead of stdio buffering each item.  */
>    size_t bufalloc = 0;
> -  bool reuse_operand_strings = true;
> -  char **operandp = operands;
> -  do
> -    {
> -      size_t operand_len = strlen (*operandp);
> -      bufalloc += operand_len + 1;
> -      if (operandp + 1 < operand_lim
> -          && *operandp + operand_len + 1 != operandp[1])
> -        reuse_operand_strings = false;
> -    }
> -  while (++operandp < operand_lim);
> +  char **operandp = argv;
> +  while (++operandp < operand_lim)
> +      bufalloc += strlen (*operandp) + 1;
>
>    /* Improve performance by using a buffer size greater than BUFSIZ / 2.
>  */
> +  bool reuse_operand_strings = true;
>    if (bufalloc <= BUFSIZ / 2)
>      {
>        bufalloc = BUFSIZ;
> --
> 2.30.2
>
>
>