Relay-Version: version B 2.10 5/3/83; site utzoo.UUCP
Posting-Version: version B 2.10.1 6/24/83; site utcsstat.UUCP
Path: utzoo!utcsstat!geoff
From: geoff@utcsstat.UUCP (Geoffrey Collyer)
Newsgroups: net.bugs.4bsd,net.unix-wizards
Subject: malloc, signal bugs in 4.2BSD RCS
Message-ID: <1827@utcsstat.UUCP>
Date: Tue, 3-Apr-84 01:21:59 EST
Article-I.D.: utcsstat.1827
Posted: Tue Apr  3 01:21:59 1984
Date-Received: Tue, 3-Apr-84 01:48:50 EST
Organization: U. of Toronto, Canada
Lines: 130

The RCS distributed with 4.2BSD contains at least two idioms of bugs:
incorrect signal catching and failure to test for success of malloc.

RCS proper and rdiff catch signals, even when run in the background.
This class of error is a v6-ism and should no longer happen,
especially since there is a tutorial on the correct way to catch
signals in UNIX Programming by Brian Kernighan and Dennis Ritchie
in Volume 2A of the UNIX Programmer's Manual.
Diffs for rcs/rcsutil.c and rdiff/diffreg.c are at the end of the article.

Nowhere in RCS proper (excluding rdiff and rdiff3) is there a test
that malloc returned a non-NULL pointer.
<flame on>
It's bad enough that some VAX programmers don't bother to free
malloc'ed memory, but not testing that malloc succeeded is just
damned unprofessional!  To paraphrase Shakespeare:

	All the world's a VAX,
	And all the coders merely butchers;
	They have their exits and their entrails;
	And one int in his time plays many widths,
	His sizeof being N bytes.  At first the infant,
	Mewling and puking in the Regent's arms.
	And then the whining schoolboy, with his Sun,
	And shining morning face, creeping like slug
	Unwillingly to school.

<flame off>
The diffs are numerous and affect nearly every source file in RCS;
it would be merely tedious to post them.  If you are running RCS
on a PDP-11 or other small machine, you can either find every call
to malloc and change it from

	foo = malloc(bar);

to

	foo = malloc(bar);
	if (foo == NULL)
		faterror("memory exhausted in <function>\n");

or contact me and send a tape (the sources are too big to mail).
In practice, the only program to have run out of memory so far
has been rlog when given an archive with a lot of deltas.

Geoff Collyer, U. of Toronto	utzoo!utcsstat!geoff
---only diffs past this point---
rcutil.c diffs:
______________
70a74,80
> int (*oldsigint)();
> int (*oldsighup)();
> int (*oldsigquit)();
> int (*oldsigpipe)();
> int (*oldsigterm)();
> char sigsread = 0;	/* if true, oldsig* have been set */
> 
188c200,201
< void catchints()
---
> void
> readsigs()
190,192c203,213
< 	signal(SIGINT,catchsig); signal(SIGHUP,catchsig);
< 	signal(SIGQUIT,catchsig); signal(SIGPIPE,catchsig);
< 	signal(SIGTERM,catchsig);
---
> 	oldsigint = signal(SIGINT, SIG_IGN);
> 	signal(SIGINT, oldsigint);
> 	oldsighup = signal(SIGHUP, SIG_IGN);
> 	signal(SIGHUP, oldsighup);
> 	oldsigquit = signal(SIGQUIT, SIG_IGN);
> 	signal(SIGQUIT, oldsigquit);
> 	oldsigpipe = signal(SIGPIPE, SIG_IGN);
> 	signal(SIGPIPE, oldsigpipe);
> 	oldsigterm = signal(SIGTERM, SIG_IGN);
> 	signal(SIGTERM, oldsigterm);
> 	sigsread = 1;
195c216,217
< void ignoreints()
---
> void
> catchints()
197,199c219,242
< 	signal(SIGINT,SIG_IGN); signal(SIGHUP,SIG_IGN);
< 	signal(SIGQUIT,SIG_IGN); signal(SIGPIPE,SIG_IGN);
< 	signal(SIGTERM,SIG_IGN);
---
> 	if (!sigsread)
> 		readsigs();
> 	if (oldsigint != SIG_IGN)	/* don't catch signal if ignoring it */
> 		signal(SIGINT, catchsig);
> 	if (oldsighup != SIG_IGN)
> 		signal(SIGHUP, catchsig);
> 	if (oldsigquit != SIG_IGN)
> 		signal(SIGQUIT, catchsig);
> 	if (oldsigpipe != SIG_IGN)
> 		signal(SIGPIPE, catchsig);
> 	if (oldsigterm != SIG_IGN)
> 		signal(SIGTERM, catchsig);
> }
> 
> void
> ignoreints()
> {
> 	if (!sigsread)
> 		readsigs();
> 	signal(SIGINT, SIG_IGN);
> 	signal(SIGHUP, SIG_IGN);
> 	signal(SIGQUIT, SIG_IGN);
> 	signal(SIGPIPE, SIG_IGN);
> 	signal(SIGTERM, SIG_IGN);
______________
diffreg.c diffs:
______________
163,166c163,170
< 	signal(SIGHUP,done);
< 	signal(SIGINT,done);
< 	signal(SIGPIPE,done);
< 	signal(SIGTERM,done);
---
> 	if (signal(SIGHUP, SIG_IGN) != SIG_IGN)
> 		signal(SIGHUP, done);
> 	if (signal(SIGINT, SIG_IGN) != SIG_IGN)
> 		signal(SIGINT, done);
> 	if (signal(SIGPIPE, SIG_IGN) != SIG_IGN)
> 		signal(SIGPIPE, done);
> 	if (signal(SIGTERM, SIG_IGN) != SIG_IGN)
> 		signal(SIGTERM, done);
______________