>Number:         9996
>Category:       bin
>Synopsis:       Program pac does not process accounting file correctly.
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Apr 27 14:15:00 PDT 2000
>Closed-Date:
>Last-Modified:
>Originator:     Brian Stark
>Release:        NetBSD-1.4X
>Organization:
	Siemens Power Transmission and Distribution
	Power Systems Control Division
>Environment:
	
System: NetBSD palomino.siemens-psc.com 1.4X NetBSD 1.4X (PALOMINO) #10: Sat Apr 22 10:49:53 CDT 2000 bstark@palomino.siemens-psc.com:/usr/src/sys/arch/i386/compile/PALOMINO i386


>Description:
	
The program pac does not process printer accounting files correctly. If you
take a look at the code in pac.c there is a function called account(). 
This code is responsible for reading the printer accounting file and 
expects data in the file to be in the following format:

	whitespace
	a float value to indicate number of feet/pages
	whitespace
	an integer to indicate the number of copies
	whitespace
	hostname
	a semi-colon
	username

(NOTE: These fields would be on one line, and not spread over multiple 
lines -- the above was done for readability)

There are several processing problems in the code. Namely, the integer
for the number of copies is only one character [0..9], and the hostname
and username are not processed correctly at all.

Although I have written some other problem reports on pac, I have not
noticed this problem until now because I have been using the unix-lpr.sh
script that comes with Ghostscript to create my accounting files, and 
I have always assumed that the unix-lpr.sh script was doing the write thing.
Now that I have seen how the code works in pac.c, I know that the unix-lpr.sh
script is not putting all of the data that it should in the printer 
accounting file.

To illustrate this, here is a sample accounting file with a slight
variation between the first and second lines for comparison in the
output:

palomino:{root}# cat acct 
   1.00 2 palomino.siemens-psc.com:root
   2.00 4palomino.siemens-psc.com:bstark
palomino:{root}# pac -P lp
 pages/feet runs price    host name and login
 ---------- ---- -------- ----------------------
       1.00    1 $   0.02 2
       2.00    1 $   0.04 4palomino.siemens-psc.com:bstark
 ---------- ---- -------- ----------------------
Sum:   3.00    2 $   0.06
palomino:{root}# 

As you can see, the count field is not being processed correctly. Because
of this, the conditional check on variable 'ic' will always fail at the
end of account() and the value for hp->h_count will only be incremented
by 1. This results in incorrect "runs" total on each line.

A side effect of this, is that the price total on each line and the price
summary is not calculated correctly.


>How-To-Repeat:
	
Create a printer accounting file like the one I created above and execute
pac -P XXX, where XXX is the printer name on your system that has the 
printer accounting file defined for it that you just created.

>Fix:

Install my source code changes to the pac.c file. My changes can be 
summarized as follows:

  * correct processing in function account()
  * correct processing in function dumpit()
  * remove function any() since it is no longer needed.

In addition, I would like to see some type of comments either in pac.c
itself or in the man page for pac describing the format of the printer
accounting file. This will help others who may try to use the 
printer accounting files to understand the correct format of the file.

Once my changes are installed, pac will report the correct information.
Here is a sample run using a larger printer accounting file:

palomino:{root}# pac -P lp 
 pages/feet runs price    host name and login
 ---------- ---- -------- ----------------------
     141.00  142 $ 400.44 palomino.siemens-psc.com:bstark
      28.00   29 $  16.24 palomino.siemens-psc.com:root
 ---------- ---- -------- ----------------------
Sum: 169.00  171 $ 577.98
palomino:{root}# pac -P lp -m 
 pages/feet runs price    login
 ---------- ---- -------- ----------------------
     141.00  142 $ 400.44 bstark
      28.00   29 $  16.24 root
 ---------- ---- -------- ----------------------
Sum: 169.00  171 $ 577.98
palomino:{root}# pac -P lp -m -p 0.01
 pages/feet runs price    login
 ---------- ---- -------- ----------------------
     141.00  142 $ 200.22 bstark
      28.00   29 $   8.12 root
 ---------- ---- -------- ----------------------
Sum: 169.00  171 $ 288.99
palomino:{root}# 


And now, here are the source code changes:

Replace the existing function account() with this one:

static void             
account(acct)   
        FILE *acct;     
{       
        const char *delimiters1 = " \t";
        const char *delimiters2 = ":";
        /*              
         * buffer needs to be able to hold a hostname!
         */
        char linebuf[MAXHOSTNAMELEN+BUFSIZ+1];
        double t;        
        char *cp, *cp2;
        struct hent *hp;
        int ic; 
        
        memset(&linebuf,'\0',sizeof(linebuf));
        while (fgets(linebuf, BUFSIZ, acct) != NULL) {
                cp = strtok(linebuf,delimiters1);
                t = atof(cp);
                
                cp = strtok(NULL,delimiters1);
                ic = atoi(cp);
  
                cp = strtok(NULL,delimiters1);
                if (mflag) {
                        cp = strtok(cp,delimiters2);
                        cp = strtok(NULL,delimiters2);
                }

                cp2 = strchr(cp,'\n');
                if (cp2)
                        (*cp2) = '\0';

                hp = lookup(cp);

                if (hp == NULL) {
                        if (!allflag)
                                continue;
                        hp = enter(cp);
                }
                hp->h_feetpages += t;
                if (ic)
                        hp->h_count += ic;
                else
                        hp->h_count++;
        }
}


Replace the existing dumpit() function with this one:

static void             
dumpit() 
{
        struct hent **base;
        struct hent *hp, **ap;
        int hno, c, runs;
        float feet; 
   
        hp = hashtab[0];
        hno = 1;
        base = (struct hent **) calloc(sizeof hp, hcount);
        if (base == NULL)
                err(1, "calloc");
        for (ap = base, c = hcount; c--; ap++) {
                while (hp == NULL)
                        hp = hashtab[hno++];
                *ap = hp;
                hp = hp->h_link;
        }       
        qsort(base, hcount, sizeof hp, qucmp);
        printf(" pages/feet runs price    %s\n",
                (mflag ? "login" : "host name and login"));
        printf(" ---------- ---- -------- ----------------------\n");
        feet = 0.0; 
        runs = 0;
        for (ap = base, c = hcount; c--; ap++) {
                hp = *ap;
                runs += hp->h_count;
                feet += hp->h_feetpages;
                printf("    %7.2f %4d $%7.2f %s\n",
                        hp->h_feetpages, hp->h_count, 
                        hp->h_feetpages * price * hp->h_count,
                        hp->h_name);
        }       
        if (allflag) {
                printf(" ---------- ---- -------- ----------------------\n");
                printf("Sum:%7.2f %4d $%7.2f\n", feet, runs, 
                        feet * price * runs);
        }
}

Remove function any(), and its prototype since it is no longer needed.

>Release-Note:
>Audit-Trail:
>Unformatted: