Re: [PATCH 3/3] tee.2: always fail after perror(), use for(;;) instead of do{}while(1)

["Alejandro Colomar (man-pages)" <alx.manpages@gmail.com>]

Hi наб,

Man pages extensively use exit(EXIT_ERROR) and exit(EXIT_SUCCESS) 
instead of return EXIT_ERROR and return EXIT_SUCCESS or even nothing at 
the end of main().  I never used that myself, and don't see much 
difference between exit(3) and return, but if only for consistency, and 
for keeping the status quo in case of doubt, I'll keep using exit(3).

However, I like the change to for(;;).  At least a while (1) would be 
sane, but do ... while (1) seems a bit weird to me :)
Could you please write a separate patch for that?

Also, please see an inline comment below.

Cheers,

Alex

On 1/3/22 16:34, наб wrote:
> ---
>   man2/tee.2 | 11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/man2/tee.2 b/man2/tee.2
> index 17b93882d..4b410ecad 100644
> --- a/man2/tee.2
> +++ b/man2/tee.2
> @@ -171,10 +171,10 @@ main(int argc, char *argv[])
>       int fd = open(argv[1], O_WRONLY | O_CREAT | O_TRUNC, 0644);
>       if (fd == \-1) {
>           perror("open");
> -        exit(EXIT_FAILURE);
> +        return EXIT_FAILURE;
>       }
>   
> -    do {
> +    for (;;) {
>           /*
>            * tee stdin to stdout.
>            */
> @@ -184,7 +184,7 @@ main(int argc, char *argv[])
>               if (errno == EAGAIN)
>                   continue;
>               perror("tee");
> -            exit(EXIT_FAILURE);
> +            return EXIT_FAILURE;
>           } else
>               if (len == 0)
>                   break;
> @@ -197,14 +197,13 @@ main(int argc, char *argv[])
>                                     len, SPLICE_F_MOVE);
>               if (slen < 0) {
>                   perror("splice");
> -                break;
> +                return EXIT_FAILURE;

This seems like a bug in the example program, so a separate patch for it 
would be better.

This makes me think that the loop wasn't originally a do ... while (1) 
but something different, and in a rewrite, a few things were forgotten, 
maybe.

>               }
>               len \-= slen;
>           }
> -    } while (1);
> +    }
>   
>       close(fd);
> -    exit(EXIT_SUCCESS);
>   }
>   .EE
>   .SH SEE ALSO

-- 
Alejandro Colomar
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/