Date: Mon, 6 Nov 2000 19:44:44 -0500 From: Garance A Drosihn <drosih@rpi.edu> To: Jeroen Ruigrok van der Werven <jruigrok@via-net-works.nl>, audit@FreeBSD.ORG, freebsd-print@bostonradio.org Subject: Fixing mktemp() call in lpd/printjob.c (logging) Message-ID: <p04330101b62cfcffd73e@[128.113.24.47]> In-Reply-To: <p0433010cb62cc872490a@[128.113.24.47]> References: <20001024140510.G93799@lucifer.bart.nl> <20001024222716.B2020@lucifer.bart.nl> <p0433010cb62cc872490a@[128.113.24.47]>
next in thread | previous in thread | raw e-mail | index | archive | help
Okay, I believe I have an update which replaces the call to mktemp with a call to mkstemp, without introducing any problems to printjob. The mktemp call was for a temporary file (in the spool directory) which was used to catch stderr output from filters. In testing this, I also noticed that this temp file was NOT copied to 'lf=' when using a filter while sending a job to a remote host (ie, rm=). The update also corrects that. I've tested this with a few dozen syslog-msgs to make sure the right things were happening with the right values at the right times. (I did remove those syslog msgs, of course... :-) So, I'm fairly confident it is doing the right thing. This does borrow some of the code Jeroen wrote in earlier attempts at this update, but moves it around and does a few other things. The update is available at: ftp://freefour.acs.rpi.edu/pub/bsdlpr/no-mktemp.diff I mention that because I'm sending the patch via Eudora on my Mac (macOS 10pb, to be precise), and I'm not quite sure how it will come out in the message. Anyway, if the following looks like garbage, then check the above URL. The copy & paste attempt: Index: lpd/printjob.c =================================================================== RCS file: /home/ncvs/src/usr.sbin/lpr/lpd/printjob.c,v retrieving revision 1.27 diff -u -r1.27 printjob.c --- lpd/printjob.c 2000/11/06 19:36:38 1.27 +++ lpd/printjob.c 2000/11/07 00:19:58 @@ -115,7 +115,8 @@ static char logname[32]; /* user's login name */ static char pxlength[10] = "-y"; /* page length in pixels */ static char pxwidth[10] = "-x"; /* page width in pixels */ -static char tempfile[] = "errsXXXXXX"; /* file name for filter errors */ +/* tempErrsfile is the filename used to catch stderr from exec-ing filters */ +static char tempErrsfile[] = "errs.XXXXXXX"; static char width[10] = "-w"; /* page width in static characters */ #define TFILENAME "fltXXXXXX" static char tfile[] = TFILENAME; /* file name for filter output */ @@ -151,8 +152,9 @@ register struct jobqueue *q, **qp; struct jobqueue **queue; register int i, nitems; - off_t pidoff; - int errcnt, count = 0; + off_t pidoff; + int errcnt, count = 0; + int tempfd; init(pp); /* set up capabilities */ (void) write(1, "", 1); /* ack that daemon is started */ @@ -169,8 +171,6 @@ signal(SIGQUIT, abortpr); signal(SIGTERM, abortpr); - (void) mktemp(tempfile); - /* * uses short form file names */ @@ -218,6 +218,21 @@ syslog(LOG_ERR, "%s: %s: %m", pp->printer, pp->lock_file); } + + /* create a file which will be used to hold stderr from filters */ + if ((tempfd = mkstemp(tempErrsfile)) == -1) { + syslog(LOG_ERR, "%s: mkstemp(%s): %m", pp->printer, + tempErrsfile); + exit(-1); + } + if ((i = fchmod(tempfd, 0664)) == -1) { + syslog(LOG_ERR, "%s: fchmod(%s): %m", pp->printer, + tempErrsfile); + exit(-1); + } + /* lpd doesn't need it to be open, it just needs it to exist */ + close(tempfd); + openpr(pp); /* open printer or remote */ again: /* @@ -314,7 +329,7 @@ } (void) close(ofd); (void) wait(NULL); - (void) unlink(tempfile); + (void) unlink(tempErrsfile); exit(0); } goto again; @@ -489,7 +504,8 @@ continue; default: /* some file to print */ - switch (i = print(pp, line[0], line+1)) { + i = print(pp, line[0], line+1); + switch (i) { case ERROR: if (bombed == OK) bombed = FATALERR; @@ -741,7 +757,9 @@ if ((child = dofork(pp, DORETURN)) == 0) { /* child */ dup2(fi, 0); dup2(fo, 1); - n = open(tempfile, O_WRONLY|O_CREAT|O_TRUNC, 0664); + /* setup stderr for the filter (child process) + * so it goes to our temporary errors file */ + n = open(tempErrsfile, O_WRONLY|O_TRUNC, 0664); if (n >= 0) dup2(n, 2); closelog(); @@ -766,8 +784,8 @@ } pp->tof = 0; - /* Copy filter output to "lf" logfile */ - if ((fp = fopen(tempfile, "r"))) { + /* Copy the filter's output to "lf" logfile */ + if ((fp = fopen(tempErrsfile, "r"))) { while (fgets(buf, sizeof(buf), fp)) fputs(buf, stderr); fclose(fp); @@ -914,6 +932,7 @@ { register int f, i, amt; struct stat stb; + FILE *fp; char buf[BUFSIZ]; int sizerr, resp, closedpr; @@ -971,8 +990,9 @@ if ((ifilter = dofork(pp, DORETURN)) == 0) { /* child */ dup2(f, 0); dup2(tfd, 1); - n = open(tempfile, O_WRONLY|O_CREAT|O_TRUNC, - TEMP_FILE_MODE); + /* setup stderr for the filter (child process) + * so it goes to our temporary errors file */ + n = open(tempErrsfile, O_WRONLY|O_TRUNC, 0664); if (n >= 0) dup2(n, 2); closelog(); @@ -989,6 +1009,13 @@ while ((pid = wait((int *)&status)) > 0 && pid != ifilter) ; + /* Copy the filter's output to "lf" logfile */ + if ((fp = fopen(tempErrsfile, "r"))) { + while (fgets(buf, sizeof(buf), fp)) + fputs(buf, stderr); + fclose(fp); + } + /* process the return-code from the filter */ switch (status.w_retcode) { case 0: break; @@ -1303,8 +1330,8 @@ cp = "NOACCT"; break; case FILTERERR: - if (stat(tempfile, &stb) < 0 || stb.st_size == 0 || - (fp = fopen(tempfile, "r")) == NULL) { + if (stat(tempErrsfile, &stb) < 0 || stb.st_size == 0 + || (fp = fopen(tempErrsfile, "r")) == NULL) { printf("\nhad some errors and may not have printed\n"); break; } @@ -1383,7 +1410,8 @@ abortpr(signo) int signo; { - (void) unlink(tempfile); + + (void) unlink(tempErrsfile); kill(0, SIGINT); if (ofilter > 0) kill(ofilter, SIGCONT); -- --- Garance Alistair Drosehn = gad@eclipse.acs.rpi.edu Senior Systems Programmer or gad@freebsd.org Rensselaer Polytechnic Institute or drosih@rpi.edu To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?p04330101b62cfcffd73e>