[PATCH] tsearch.3: Simplify type usage and remove unneeded casts

[PATCH] tsearch.3: Simplify type usage and remove unneeded casts

[Alejandro Colomar]

The type of `var` is `int **`, and it will work with tsearch()
anyway because of implicit cast from `void *`, so declaring it as an
`int **` simplifies the code.

Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
---
 man3/tsearch.3 | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/man3/tsearch.3 b/man3/tsearch.3
index 32ddb8127..65fcadc52 100644
--- a/man3/tsearch.3
+++ b/man3/tsearch.3
@@ -323,8 +323,7 @@ action(const void *nodep, VISIT which, int depth)
 int
 main(void)
 {
-    int i, *ptr;
-    void *val;
+    int i, *ptr, **val;
 
     srand(time(NULL));
     for (i = 0; i < 12; i++) {
@@ -333,7 +332,7 @@ main(void)
         val = tsearch((void *) ptr, &root, compare);
         if (val == NULL)
             exit(EXIT_FAILURE);
-        else if ((*(int **) val) != ptr)
+        else if (*val != ptr)
             free(ptr);
     }
     twalk(root, action);
-- 
2.28.0

Re: [PATCH] tsearch.3: Simplify type usage and remove unneeded casts

[Michael Kerrisk (man-pages)]

Hi Alex,

On 9/5/20 12:50 PM, Alejandro Colomar wrote:
> The type of `var` is `int **`, and it will work with tsearch()
> anyway because of implicit cast from `void *`, so declaring it as an
> `int **` simplifies the code.
> 
> Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
> ---
>  man3/tsearch.3 | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

By chance, I've just made a conflicting change, but also...

> diff --git a/man3/tsearch.3 b/man3/tsearch.3
> index 32ddb8127..65fcadc52 100644
> --- a/man3/tsearch.3
> +++ b/man3/tsearch.3
> @@ -323,8 +323,7 @@ action(const void *nodep, VISIT which, int depth)
>  int
>  main(void)
>  {
> -    int i, *ptr;
> -    void *val;
> +    int i, *ptr, **val;

Not quite your fault, since you followed an already poor example, 
but many people (and I am one of them) frown on declarations lines 
such as the above: 'int', 'int *' and 'int **' are three different
types, and it's considered bad form to declare variables with
different type in one source line. (It's very easy to overlook
an asterisk or two when scanning the source.) Better is:

int i
int *ptr;
int **val;

>  
>      srand(time(NULL));
>      for (i = 0; i < 12; i++) {
> @@ -333,7 +332,7 @@ main(void)
>          val = tsearch((void *) ptr, &root, compare);
>          if (val == NULL)
>              exit(EXIT_FAILURE);
> -        else if ((*(int **) val) != ptr)
> +        else if (*val != ptr)
>              free(ptr);
>      }
>      twalk(root, action);

Could you please recraft this patch against current Git, which 
changed in the last minutes...

Thanks,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH] tsearch.3: Simplify type usage and remove unneeded casts

[Alejandro Colomar]

Hi Michael,

On 9/5/20 5:35 PM, Michael Kerrisk (man-pages) wrote:
> By chance, I've just made a conflicting change, but also...

I noticed while rebasing, and was already preparing the fix :)

>> -    int i, *ptr;
>> -    void *val;
>> +    int i, *ptr, **val;
>
> Not quite your fault, since you followed an already poor example,
> but many people (and I am one of them) frown on declarations lines
> such as the above: 'int', 'int *' and 'int **' are three different
> types, and it's considered bad form to declare variables with
> different type in one source line. (It's very easy to overlook
> an asterisk or two when scanning the source.) Better is:
>
> int i
> int *ptr;
> int **val;

Agree.  I also do that in my code, but was following the existing
practice.

> Could you please recraft this patch against current Git, which
> changed in the last minutes...

Sure.

Regards,
Alex.

[PATCH v2] tsearch.3: Simplify type usage and remove unneeded casts

[Alejandro Colomar]

The type of `val` is `int **`, and it will work with tsearch()
anyway because of implicit cast from `void *`, so declaring it as an
`int **` simplifies the code.

Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
---
 man3/tsearch.3 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/man3/tsearch.3 b/man3/tsearch.3
index 2e8403130..7b82d9bd3 100644
--- a/man3/tsearch.3
+++ b/man3/tsearch.3
@@ -323,7 +323,7 @@ action(const void *nodep, VISIT which, int depth)
 int
 main(void)
 {
-    void *val;
+    int **val;
 
     srand(time(NULL));
     for (int i = 0; i < 12; i++) {
@@ -332,7 +332,7 @@ main(void)
         val = tsearch((void *) ptr, &root, compare);
         if (val == NULL)
             exit(EXIT_FAILURE);
-        else if ((*(int **) val) != ptr)
+        else if (*val != ptr)
             free(ptr);
     }
     twalk(root, action);
-- 
2.28.0

Re: [PATCH v2] tsearch.3: Simplify type usage and remove unneeded casts

[Michael Kerrisk (man-pages)]

Hello Alex,

On 9/5/20 6:10 PM, Alejandro Colomar wrote:
> The type of `val` is `int **`, and it will work with tsearch()
> anyway because of implicit cast from `void *`, so declaring it as an
> `int **` simplifies the code.

Thanks, patch applied.

Cheers,

Michael

> Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
> ---
>  man3/tsearch.3 | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/man3/tsearch.3 b/man3/tsearch.3
> index 2e8403130..7b82d9bd3 100644
> --- a/man3/tsearch.3
> +++ b/man3/tsearch.3
> @@ -323,7 +323,7 @@ action(const void *nodep, VISIT which, int depth)
>  int
>  main(void)
>  {
> -    void *val;
> +    int **val;
>  
>      srand(time(NULL));
>      for (int i = 0; i < 12; i++) {
> @@ -332,7 +332,7 @@ main(void)
>          val = tsearch((void *) ptr, &root, compare);
>          if (val == NULL)
>              exit(EXIT_FAILURE);
> -        else if ((*(int **) val) != ptr)
> +        else if (*val != ptr)
>              free(ptr);
>      }
>      twalk(root, action);
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

AW: [PATCH v2] tsearch.3: Simplify type usage and remove unneeded casts

[Walter Harms]

Hello,
i am sorry to interrupt here but ...
IMHO the void *val is here for a reason, because it means
"this can be anything" the reason why int ** works here is
that the example uses int. You make the example to specific
in this case. may be the example from bsearch is better here.



________________________________________

Von: linux-man-owner@vger.kernel.org [linux-man-owner@vger.kernel.org] im Auftrag von Michael Kerrisk (man-pages) [mtk.manpages@gmail.com]
Gesendet: Samstag, 5. September 2020 21:42
An: Alejandro Colomar
Cc: mtk.manpages@gmail.com; linux-man@vger.kernel.org
Betreff: Re: [PATCH v2] tsearch.3: Simplify type usage and remove unneeded casts

Hello Alex,

On 9/5/20 6:10 PM, Alejandro Colomar wrote:
> The type of `val` is `int **`, and it will work with tsearch()
> anyway because of implicit cast from `void *`, so declaring it as an
> `int **` simplifies the code.

Thanks, patch applied.

Cheers,

Michael

> Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
> ---
>  man3/tsearch.3 | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/man3/tsearch.3 b/man3/tsearch.3
> index 2e8403130..7b82d9bd3 100644
> --- a/man3/tsearch.3
> +++ b/man3/tsearch.3
> @@ -323,7 +323,7 @@ action(const void *nodep, VISIT which, int depth)
>  int
>  main(void)
>  {
> -    void *val;
> +    int **val;
>
>      srand(time(NULL));
>      for (int i = 0; i < 12; i++) {
> @@ -332,7 +332,7 @@ main(void)
>          val = tsearch((void *) ptr, &root, compare);
>          if (val == NULL)
>              exit(EXIT_FAILURE);
> -        else if ((*(int **) val) != ptr)
> +        else if (*val != ptr)
>              free(ptr);
>      }
>      twalk(root, action);
>


--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: AW: [PATCH v2] tsearch.3: Simplify type usage and remove unneeded casts

[Alejandro Colomar]

Hello Walter,

On 2020-09-07 09:54, Walter Harms wrote:
> Hello,
> i am sorry to interrupt here but ...
> IMHO the void *val is here for a reason, because it means
> "this can be anything" the reason why int ** works here is
> that the example uses int. You make the example to specific
> in this case. may be the example from bsearch is better here.

I would say the example isn't clearer with `void *` and the cast; I see 
it OK as it is now.  But let's see what others think about it...

Cheers,

Alex.