[comp.soft-sys.andrew] Simple fix to core dump bug in AMS

nsb@THUMPER.BELLCORE.COM (Nathaniel Borenstein) (02/12/90)

Running on a Sun4 with patchlevel 2 (no, I haven't gotten around to
patch 3 yet) and NO AFS, all messagserver clients (cui, messages, vui)
will repeatably dump core on zero-length files in a Mailbox directory.

The bug is extremely easy to fix.  The fix is on line 523 of
ams/libs/ms/init.c.  The old verison of that line is

    if (AMS_GecosHacks && ULstrcmp(domain, ThisDomain) == 0) {

All you need to do is to make it slightly more robust by adding a couple
of null pointer checks, to wit:

    if (AMS_GecosHacks && domain && ThisDomain && ULstrcmp(domain,
ThisDomain) == 0) {

(Actually, you could probably get away with checking only "domain", but
I figured it couldn't hurt to check ThisDomain while you're at it.  This
code only runs on zero-length files, so an extra check doesn't cost you
anything in the normal case.)

This turns out to become relevant once you're serious about a
large-scale AMS bboard system without AFS.  Cheers.  -- Nathaniel

cfe@TRANSARC.COM (Craig Everhart) (02/13/90)

I'd make a better fix to the problem that Nathaniel discusses.

Yes, the changes he suggests will prevent a core dump.  I would argue
that they don't necessarily do the right thing in the cases that would
have dumped core.

There are several callers of this code, not just the place in rawfile.c
where zero-length files are handled.  All other callers, however, always
ensure that the ``domain'' argument is non-null, and the call in
rawfile.c at present will always pass a ``domain'' argument when the
file in question is being read from AFS.

Nathaniel's fix is fine as a point of defense, but I'd suggest the
following to get correct semantics.  In ams/libs/ms/rawfile.c, line 132
in my copy, is a block of code that looks like the following:

	if (IsOnVice(fdtemp) && NewMessage->AuthCell) {
	    p = getcpwuid(statbuf.st_uid, NewMessage->AuthCell);
	} else {
	    p = getpwuid(statbuf.st_uid);
	}
	if (p) {
	    GetNameFromGecos(p->pw_gecos, p->pw_name, NewMessage->AuthCell, &uname);
	}

The fix I'd suggest is to replace that with:

	if (IsOnVice(fdtemp) && NewMessage->AuthCell) {
	    p = getcpwuid(statbuf.st_uid, NewMessage->AuthCell);
	    if (p) {
		GetNameFromGecos(p->pw_gecos, p->pw_name, NewMessage->AuthCell, &uname);
	    }
	} else {
	    p = getpwuid(statbuf.st_uid);
	    if (p) {
		GetNameFromGecos(p->pw_gecos, p->pw_name, MyMailDomain, &uname);
	    }
	}



		Craig