Path: utzoo!utgpu!jarvis.csri.toronto.edu!rutgers!njin!princeton!notecnirp!nfs
From: nfs@notecnirp.Princeton.EDU (Norbert Schlenker)
Newsgroups: comp.os.minix
Subject: Zen and the Art of Library Programming
Summary: Something to gab about
Message-ID: <22630@princeton.Princeton.EDU>
Date: 27 Dec 89 05:47:50 GMT
Sender: news@princeton.Princeton.EDU
Reply-To: nfs@notecnirp.UUCP (Norbert Schlenker)
Organization: Dept. of Computer Science, Princeton University
Lines: 151

I think that a philosophical point has to be discussed, and the recent
posting of v1.5 makes me think that now is a good time.  I'll get to the
philosophy at the end; at the moment, let me motivate the discussion.

Consider the recent implementation of assert.c (a helper function for
the assert() macro defined in <assert.h>).  As posted, it reads thus:

----------------------------------------------------------------------------
#include <lib.h>
/* This is the routine called by <assert.h>. */

#include <stdio.h>
#include <stdlib.h>

void __assert(file, line)
char *file;
long line;
{
  fprintf(stderr, "Assertion error in file %s on line %u\n", file, line);
  abort();
}
----------------------------------------------------------------------------

What's wrong with this routine?  There is a problem with the declaration
of "line", which appears here as a long but which is likely to be compiled
as an int in the calling routine (the PC Minix 1.3 compiler does this).

But that's not the major problem at all.  The BIG problem is that __assert()
calls fprintf() and abort() and I assert that this is NOT A GOOD THING!

For one thing, it is possible to write an ANSI C conforming program which
will not work with this definition of __assert().  Imagine:

----------------------------------------------------------------------------
#include <assert.h>
#include <stdio.h>

abort()
{
  while (1)
	fprintf(stderr, "Stupid jerk!\n");
}

main()
{
  assert(0==1);
}
----------------------------------------------------------------------------

This is a valid ANSI C program which should terminate with a SIGABRT signal.
Instead, the linker will use the program's definition of abort(), and this
program will go into an infinite loop when run.  (Just as an aside, this is
a useful test program for many compilers that claim ANSI compatibility.)

It can be argued that this is a rather contrived example, and I suppose it is.
But consider a piece of a rather more reasonable program and a situation that
I have actually run into when porting to Minix:

----------------------------------------------------------------------------
long int len(path)
char *path;
{
/* some baloney */
  return (long) buf.st_size;
}

... more functions, some of which call len() ...
----------------------------------------------------------------------------

This won't link under PC Minix 1.3.  Why?  Because buried in lib/call.c is
a definition of a function called len(), and call.s is going to be linked
into any non-trivial Minix program.  The name conflict causes the link to
fail.  The len() function in the program has to be renamed, for no good
reason other than a sloppy implementation of the Minix library.

Similar problems are easy to find.  Fwrite() in <stdio> uses write() [from
POSIX <unistd>], but the program will probably fail if a program has a 
function called write().  Calloc() calls malloc() - what if a program 
supplies its own?  Malloc calls brk() - what if a program supplies its own?
Such programs are perfectly possible and perfectly legal.  They will not
work under Minix and I think that something has to be done to fix this up.

Here is my suggestion.  It's not pretty, and it is going to add a procedure
call layer onto a path that is probably already too long.  But it will work,
and I don't think anything simpler will do the trick.

Every (!) library function becomes a simple front-end for another function
which does the work.  Every library function can call ONLY the underlying
functions (I like to call them underware :-)  To avoid ANSI namespace
conflicts, I suggest the underware's names be prefixed with two underscores.
Definitions to aid in this should be added to <lib.h>.  To make this a
little more concrete, let's try the following:

--------------------------------- lib.h ------------------------------------
...
#define LIB_ABORT	__abort
#define LIB_GETPID	__getpid
#define LIB_KILL	__kill
...
-------------------------------- abort.c -----------------------------------
/* abort.c						ANSI 4.10.4.1
 *	void abort(void);
 *
 *	Causes abnormal program termination by raising SIGABRT.
 */

#include <stdlib.h>
#include "lib.h"

#ifdef abort
#undef abort
#endif

PUBLIC void abort()
{
  LIB_ABORT();
}
-------------------------------- ABORT.c -----------------------------------
/* ABORT.c						Implementation
 *	void LIB_ABORT(void);
 *
 *	Provides underlying functionality for ANSI abort().
 */

#include <signal.h>
#include "lib.h"

PUBLIC void LIB_ABORT()
{
  LIB_KILL(LIB_GETPID(), SIGABRT);
}
------------------------------- GETPID.c -----------------------------------
-------------------------------- KILL.c ------------------------------------
etc. etc.
----------------------------------------------------------------------------

See how abort() works?  Now we can fix __assert() pretty easily too - rather
than calling abort(), we make it call LIB_ABORT() (i.e. __abort()) instead.
(Fprintf() is another problem, but we can fix that similarly.)  No program
that follows ANSI dictates can mess up this scheme.  No other reasonable
program should :-)

I believe that the long run benefits of this approach outweigh the costs of
doing it once correctly.  I am going to do something like this for myself,
no matter what.  I can even see a simple method, using the peephole optimizer
posted once upon a time, of removing virtually all the space and time overhead
that this method entails.

Does anyone have any comments and/or suggestions?

Norbert