geoff@utcsstat.UUCP (Geoffrey Collyer) (04/03/84)
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);
______________