Subject: bin/3634: irritating little bug in lastcomm
To: None <gnats-bugs@gnats.netbsd.org>
From: John F. Woods <jfw@jfwhome.funhouse.com>
List: netbsd-bugs
Date: 05/16/1997 22:54:03
>Number:         3634
>Category:       bin
>Synopsis:       lastcomm seeks to negative address
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    bin-bug-people (Utility Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri May 16 20:05:01 1997
>Last-Modified:
>Originator:     John F. Woods
>Organization:
Misanthropes-R-Us
>Release:        May 6, 1997
>Environment:
	
System: NetBSD jfwhome.funhouse.com 1.2D NetBSD 1.2D (DMA-jfw) #4: Thu May 8 17:12:52 PDT 1997 thorpej@dracul:/u5/netbsd/src/sys/arch/i386/compile/DMA-jfw i386


>Description:
	I noticed recently that lastcomm prints "Invalid argument" before
exiting.  I don't know why it hasn't always done that; perhaps lseek only
recently began returning an error when you seek to -40 -- for that is what
lastcomm does.

	lastcomm reads the accounting file backward by seeking to the
last record in the file (rounded filesize minus the size of the accounting
record, 40 bytes on the i386), then it loops:  read the item, fseek() before
the one read and one prior to that (2 * -(long)sizeof(struct acct), SEEK_CUR),
bust out of the loop if the variable "size" is 0, else decrease size by
sizeof(struct acct) and see if we should print the current entry.

	However, if we have just read the first record in the file, seeking
backward two records puts us before the beginning of the file.  fseek()
returns an error (is that a recent change?), which causes a message instead
of processing the first record.

>How-To-Repeat:
	"lastcomm root | head" should demonstrate it adequately.

>Fix:
I have rearranged the seek-and-test logic as follows.  Perhaps the original
logic was attempting to avoid processing the first record (which is always
the "sa" command); if that's desirable, this should be changed a bit.  (For
example, make the test at the bottom "if ( (size -= sizeof(struct acct)) == 0)"
if you don't find that tasteless. :-) )  However, exiting with an error is
certainly not the right way to go about it.

*** lastcomm.c.ORIG	Mon Dec 11 15:50:02 1995
--- lastcomm.c	Fri May 16 22:38:26 1997
***************
*** 124,136 ****
  		if (fread(&ab, sizeof(struct acct), 1, fp) != 1)
  			err(1, "%s", acctfile);
  
- 		if (fseek(fp, 2 * -(long)sizeof(struct acct), SEEK_CUR) == -1)
- 			err(1, "%s", acctfile);
- 
- 		if (size == 0)
- 			break;
- 		size -= sizeof(struct acct);
- 
  		if (ab.ac_comm[0] == '\0') {
  			ab.ac_comm[0] = '?';
  			ab.ac_comm[1] = '\0';
--- 124,129 ----
***************
*** 139,159 ****
  			    p < &ab.ac_comm[fldsiz(acct, ac_comm)] && *p; ++p)
  				if (!isprint(*p))
  					*p = '?';
! 		if (*argv && !requested(argv, &ab))
! 			continue;
  
! 		t = expand(ab.ac_utime) + expand(ab.ac_stime);
! 		(void)printf(
!                     "%-*.*s %-7s %-*.*s %-*.*s %6.2f secs %.16s",
! 		    fldsiz(acct, ac_comm), fldsiz(acct, ac_comm), ab.ac_comm,
! 		    flagbits(ab.ac_flag), UT_NAMESIZE, UT_NAMESIZE,
! 		    user_from_uid(ab.ac_uid, 0), UT_LINESIZE, UT_LINESIZE,
! 		    getdev(ab.ac_tty), t / (double)AHZ, ctime(&ab.ac_btime));
! 		delta = expand(ab.ac_etime) / (double)AHZ;
!  	        printf(" (%1.0lf:%02.0lf:%05.2lf)\n",
! 		    delta / SECSPERHOUR,
! 		    fmod(delta, SECSPERHOUR) / SECSPERMIN,
! 		    fmod(delta, SECSPERMIN));
  	}
  	exit(0);
  }
--- 132,160 ----
  			    p < &ab.ac_comm[fldsiz(acct, ac_comm)] && *p; ++p)
  				if (!isprint(*p))
  					*p = '?';
! 		if (!*argv || requested(argv, &ab)) {
  
! 			t = expand(ab.ac_utime) + expand(ab.ac_stime);
! 			(void)printf(
! 				     "%-*.*s %-7s %-*.*s %-*.*s %6.2f secs %.16s",
! 				     fldsiz(acct, ac_comm), fldsiz(acct, ac_comm), ab.ac_comm,
! 				     flagbits(ab.ac_flag), UT_NAMESIZE, UT_NAMESIZE,
! 				     user_from_uid(ab.ac_uid, 0), UT_LINESIZE, UT_LINESIZE,
! 				     getdev(ab.ac_tty), t / (double)AHZ, ctime(&ab.ac_btime));
! 			delta = expand(ab.ac_etime) / (double)AHZ;
! 			printf(" (%1.0lf:%02.0lf:%05.2lf)\n",
! 			       delta / SECSPERHOUR,
! 			       fmod(delta, SECSPERHOUR) / SECSPERMIN,
! 			       fmod(delta, SECSPERMIN));
! 		}
! 		/* are we at the beginning of the file yet? */
! 		if (size == 0)
! 			break;
! 		/* seek backward over the one we read and the next to read */
! 		if (fseek(fp, 2 * -(long)sizeof(struct acct), SEEK_CUR) == -1)
! 			err(1, "%s", acctfile);
! 		/* and account for its size */
! 		size -= sizeof(struct acct);
  	}
  	exit(0);
  }

>Audit-Trail:
>Unformatted: