Re: bin/53996: tip(1) fails to handle EOF on stdin

[Andreas Gustafsson <gson%gson.org@localhost>]

The following reply was made to PR bin/53996; it has been noted by GNATS.

From: Andreas Gustafsson <gson%gson.org@localhost>
To: Valery Ushakov <uwe%stderr.spb.ru@localhost>
Cc: gnats-bugs%NetBSD.org@localhost
Subject: Re: bin/53996: tip(1) fails to handle EOF on stdin
Date: Sat, 23 Feb 2019 12:25:29 +0200

 Valery Ushakov wrote:
 > To reproduce a bug / test a fix, what else do you usually need test
 > cases for :)
 
 Calling getchar() without checking for EOF is always a bug, whether or
 not it can be demonstrated with a test case, and it also seems highly
 unlikely that adding the check for EOF would fail to fix the bug.
 But the important thing is that it's fixed - thanks!
 
 I do have a couple of style nits, though...
 
 The cast to char in in "return (char)c & STRIP_PAR;" is pointless;
 the result is immediately promoted back to int anyway for the "&"
 operation, and then converted to char again for the return.
 
 Also, I think it would be cleaner for xgetchar() to have the same
 return type as getchar().  It doesn't matter to the existing callers
 in tip, but in general, having a getchar() wrapper return char is bad
 because it will then return negative values for characters with the
 8th bit set on systems where char is signed, whereas getchar() itself
 will not.
 
 That is, I propose the following changes:
 
 Index: tip.c
 ===================================================================
 RCS file: /bracket/repo/src/usr.bin/tip/tip.c,v
 retrieving revision 1.61
 diff -u -r1.61 tip.c
 --- tip.c	22 Feb 2019 22:25:22 -0000	1.61
 +++ tip.c	23 Feb 2019 10:17:30 -0000
 @@ -332,7 +332,7 @@
  /*
   * getchar() wrapper that checks for EOF on the local end.
   */
 -static char
 +static int
  xgetchar(void)
  {
  	int c = getchar();
 @@ -341,7 +341,7 @@
  		/* NOTREACHED */
  	}
  
 -	return (char)c & STRIP_PAR;
 +	return c & STRIP_PAR;
  }
  
 > Let me know if I can request the pull-ups.
 
 Fine by me (with or without the above patch).
 -- 
 Andreas Gustafsson, gson%gson.org@localhost