Subject: Re: Runaway yes(1)
To: Jed Davis <jdev@panix.com>
From: Roland Illig <rillig@NetBSD.org>
List: tech-userlevel
Date: 11/18/2005 08:58:22
This is a multi-part message in MIME format.
--------------090003000500090403060906
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

Jed Davis wrote:
> I feel that yes(1) should stop trying to write its message to stdout
> if it encounters an error, rather than spinning forever accomplishing
> nothing.
> 
> This can occur if it's run in a pipeline with SIGPIPE ignored, perhaps
> because some ancestor process set it that way to do socket I/O and
> failed to reenable it before exec'ing.  (Yes, such programs should be
> fixed, but that's not the point.)  A similar effect could be
> encountered with a revoked tty.

I didn't know yet how signals are passed by execve(2). Thanks for 
clarifying this.

 From the execve(2) man page:
> Signals set to be ignored in the calling process are set to be 
> ignored in the new process. Signals which are set to be caught in the
> calling process image are set to default action in the new process
> image. Blocked signals remain blocked regardless of changes to the
> signal action. The signal stack is reset to be undefined (see
> sigaction(2) for more information).

> No, this isn't terribly important, but I've attached a (tested, of
> course) patch, which also fixes some style issues; so, all I need is
> for someone with sufficient authority to say that I should or
> shouldn't commit it.

I don't have the authority, but I would like it to be committed, after a 
tiny change.

> Index: yes.c
> ===================================================================
> RCS file: /cvsroot/src/usr.bin/yes/yes.c,v
> retrieving revision 1.6
> diff -u -r1.6 yes.c
> --- yes.c	7 Aug 2003 11:17:56 -0000	1.6
> +++ yes.c	18 Nov 2005 02:47:04 -0000
> @@ -43,17 +43,17 @@
>  #endif /* not lint */
>  
>  #include <stdio.h>
> -
> -int main __P((int, char **));
> +#include <stdlib.h>
>  
>  int
> -main(argc, argv)
> -	int argc;
> -	char **argv;
> +main(int argc, char *argv[])
>  {

The style guide expects another empty line here, to make it visually 
obvious that a function does not have local variables.

>  	if (argc > 1)
> -		for(;;)
> -			(void)puts(argv[1]);
> -	else for (;;)
> -		(void)puts("y");
> +		while (puts(argv[1]) >= 0)
> +			continue;
> +	else 
> +		while (puts("y") >= 0)
> +			continue;
> +
> +	return EXIT_FAILURE;

I don't like code duplication. Maybe you like my appended patch, too.

Roland

--------------090003000500090403060906
Content-Type: text/plain;
 name="yes.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="yes.patch"

Index: Makefile
===================================================================
RCS file: /cvsroot/src/usr.bin/yes/Makefile,v
retrieving revision 1.3
diff -u -p -r1.3 Makefile
--- Makefile	14 Nov 1994 04:56:13 -0000	1.3
+++ Makefile	18 Nov 2005 07:54:32 -0000
@@ -2,5 +2,6 @@
 #	$NetBSD: Makefile,v 1.3 1994/11/14 04:56:13 jtc Exp $
 
 PROG=	yes
+WARNS=	4
 
 .include <bsd.prog.mk>
Index: yes.c
===================================================================
RCS file: /cvsroot/src/usr.bin/yes/yes.c,v
retrieving revision 1.6
diff -u -p -r1.6 yes.c
--- yes.c	7 Aug 2003 11:17:56 -0000	1.6
+++ yes.c	18 Nov 2005 07:54:32 -0000
@@ -43,17 +43,17 @@ __RCSID("$NetBSD: yes.c,v 1.6 2003/08/07
 #endif /* not lint */
 
 #include <stdio.h>
-
-int main __P((int, char **));
+#include <stdlib.h>
 
 int
-main(argc, argv)
-	int argc;
-	char **argv;
+main(int argc, char **argv)
 {
-	if (argc > 1)
-		for(;;)
-			(void)puts(argv[1]);
-	else for (;;)
-		(void)puts("y");
+	const char *yes;
+
+	yes = (argc > 1) ? argv[1] : "y";
+
+	while (puts(yes) >= 0)
+		continue;
+
+	return EXIT_FAILURE;
 }

--------------090003000500090403060906--