From owner-freebsd-audit Sun Jun 2 6:58:36 2002 Delivered-To: freebsd-audit@freebsd.org Received: from flood.ping.uio.no (flood.ping.uio.no [129.240.78.31]) by hub.freebsd.org (Postfix) with ESMTP id E454837B406 for ; Sun, 2 Jun 2002 06:58:32 -0700 (PDT) Received: by flood.ping.uio.no (Postfix, from userid 2602) id 725835307; Sun, 2 Jun 2002 15:58:31 +0200 (CEST) X-URL: http://www.ofug.org/~des/ X-Disclaimer: The views expressed in this message do not necessarily coincide with those of any organisation or company with which I am or have been affiliated. To: Bruce Evans Cc: Dima Dorfman , Subject: Re: %j for printf(9) References: <20020601233017.Q2458-100000@gamplex.bde.org> From: Dag-Erling Smorgrav Date: 02 Jun 2002 15:58:30 +0200 In-Reply-To: <20020601233017.Q2458-100000@gamplex.bde.org> Message-ID: Lines: 41 User-Agent: Gnus/5.0808 (Gnus v5.8.8) Emacs/21.2 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Bruce Evans writes: > When splitting this, you might consider fixing he disorder in it (but fixing > the external disorder and misarrangement of the other declarations of ints > should be in a separate patch). Removed > Unrelated style fix :-). Removed > Another unrelated style fix. This one makes the patch harder to read. Removed > Please remove the 'n' case, and check this a bit. I forgot to remove it > soon after rev.1.48. Done. It wasn't used anywhere in the kernel. I also took the liberty of implementing the "real" %n: case 'n': if (jflag) *(va_arg(ap, intmax_t *)) = retval; else if (qflag) *(va_arg(ap, quad_t *)) = retval; else if (lflag) *(va_arg(ap, long *)) = retval; else *(va_arg(ap, int *)) = retval; break; > This seems to print all the garbage for %. It might be useful > to mark up the garbage. But gcc will detect the garbage at compile time > for literal strings. The original code would print different, possibly incorrect garbage. DES -- Dag-Erling Smorgrav - des@ofug.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Sun Jun 2 7: 0:55 2002 Delivered-To: freebsd-audit@freebsd.org Received: from flood.ping.uio.no (flood.ping.uio.no [129.240.78.31]) by hub.freebsd.org (Postfix) with ESMTP id 76DE537B404 for ; Sun, 2 Jun 2002 07:00:53 -0700 (PDT) Received: by flood.ping.uio.no (Postfix, from userid 2602) id 057E65307; Sun, 2 Jun 2002 16:00:51 +0200 (CEST) X-URL: http://www.ofug.org/~des/ X-Disclaimer: The views expressed in this message do not necessarily coincide with those of any organisation or company with which I am or have been affiliated. To: Bruce Evans Cc: Dima Dorfman , Subject: Re: %j for printf(9) References: <20020601232049.E2458-100000@gamplex.bde.org> From: Dag-Erling Smorgrav Date: 02 Jun 2002 16:00:50 +0200 In-Reply-To: <20020601232049.E2458-100000@gamplex.bde.org> Message-ID: Lines: 13 User-Agent: Gnus/5.0808 (Gnus v5.8.8) Emacs/21.2 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Bruce Evans writes: > It came out 1 line shorter with less duplication for me. Here is a patch > relative to the version in your next mail. I didn't rename fetch* to > handle*, and I may have broken something moved the initializations of > `sign' around too much. I reordered some initializations (especially for > %p) so that the order is almost (?) always (base, ..., sign, num). Applied, and renamed fetch_* to handle_*. I'll test it a bit before committing. DES -- Dag-Erling Smorgrav - des@ofug.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Sun Jun 2 7:19:20 2002 Delivered-To: freebsd-audit@freebsd.org Received: from flood.ping.uio.no (flood.ping.uio.no [129.240.78.31]) by hub.freebsd.org (Postfix) with ESMTP id 0A49A37B403 for ; Sun, 2 Jun 2002 07:19:07 -0700 (PDT) Received: by flood.ping.uio.no (Postfix, from userid 2602) id 5492E5307; Sun, 2 Jun 2002 16:19:04 +0200 (CEST) X-URL: http://www.ofug.org/~des/ X-Disclaimer: The views expressed in this message do not necessarily coincide with those of any organisation or company with which I am or have been affiliated. To: Bruce Evans Cc: Dima Dorfman , Subject: Re: %j for printf(9) References: <20020601232049.E2458-100000@gamplex.bde.org> From: Dag-Erling Smorgrav Date: 02 Jun 2002 16:19:04 +0200 In-Reply-To: Message-ID: Lines: 12 User-Agent: Gnus/5.0808 (Gnus v5.8.8) Emacs/21.2 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG --=-=-= Dag-Erling Smorgrav writes: > Applied, and renamed fetch_* to handle_*. I'll test it a bit before > committing. Here's the full patch, btw. DES -- Dag-Erling Smorgrav - des@ofug.org --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=subr_prf.diff Index: subr_prf.c =================================================================== RCS file: /home/ncvs/src/sys/kern/subr_prf.c,v retrieving revision 1.81 diff -u -r1.81 subr_prf.c --- subr_prf.c 29 Apr 2002 09:15:38 -0000 1.81 +++ subr_prf.c 2 Jun 2002 14:00:03 -0000 @@ -48,6 +48,7 @@ #include #include #include +#include #include #include #include @@ -65,7 +66,7 @@ #define TOLOG 0x04 /* Max number conversion buffer length: a u_quad_t in base 2, plus NUL byte. */ -#define MAXNBUF (sizeof(quad_t) * NBBY + 1) +#define MAXNBUF (sizeof(intmax_t) * NBBY + 1) struct putchar_arg { int flags; @@ -86,8 +87,7 @@ static void msglogchar(int c, int pri); static void msgaddchar(int c, void *dummy); static void putchar(int ch, void *arg); -static char *ksprintn(char *nbuf, u_long num, int base, int *len); -static char *ksprintqn(char *nbuf, u_quad_t num, int base, int *len); +static char *ksprintn(char *nbuf, uintmax_t num, int base, int *len); static void snprintf_func(int ch, void *arg); static int consintr = 1; /* Ok to handle console interrupts? */ @@ -426,9 +426,9 @@ * The buffer pointed to by `nbuf' must have length >= MAXNBUF. */ static char * -ksprintn(nbuf, ul, base, lenp) +ksprintn(nbuf, num, base, lenp) char *nbuf; - u_long ul; + uintmax_t num; int base, *lenp; { char *p; @@ -436,26 +436,8 @@ p = nbuf; *p = '\0'; do { - *++p = hex2ascii(ul % base); - } while (ul /= base); - if (lenp) - *lenp = p - nbuf; - return (p); -} -/* ksprintn, but for a quad_t. */ -static char * -ksprintqn(nbuf, uq, base, lenp) - char *nbuf; - u_quad_t uq; - int base, *lenp; -{ - char *p; - - p = nbuf; - *p = '\0'; - do { - *++p = hex2ascii(uq % base); - } while (uq /= base); + *++p = hex2ascii(num % base); + } while (num /= base); if (lenp) *lenp = p - nbuf; return (p); @@ -491,19 +473,18 @@ kvprintf(char const *fmt, void (*func)(int, void*), void *arg, int radix, va_list ap) { #define PCHAR(c) {int cc=(c); if (func) (*func)(cc,arg); else *d++ = cc; retval++; } - char nbuf[MAXNBUF]; - char *p, *q, *d; + char nbuf[MAXNBUF], *d; + const char *percent, *p, *q; u_char *up; int ch, n; - u_long ul; - u_quad_t uq; + uintmax_t num; int base, lflag, qflag, tmp, width, ladjust, sharpflag, neg, sign, dot; + int jflag; int dwidth; char padc; int retval = 0; - ul = 0; - uq = 0; + num = 0; if (!func) d = (char *) arg; else @@ -523,8 +504,10 @@ return (retval); PCHAR(ch); } + percent = fmt - 1; qflag = 0; lflag = 0; ladjust = 0; sharpflag = 0; neg = 0; sign = 0; dot = 0; dwidth = 0; + jflag = 0; reswitch: switch (ch = (u_char)*fmt++) { case '.': dot = 1; @@ -571,17 +554,17 @@ width = n; goto reswitch; case 'b': - ul = va_arg(ap, int); + num = va_arg(ap, int); p = va_arg(ap, char *); - for (q = ksprintn(nbuf, ul, *p++, NULL); *q;) + for (q = ksprintn(nbuf, num, *p++, NULL); *q;) PCHAR(*q--); - if (!ul) + if (num == 0) break; for (tmp = 0; *p;) { n = *p++; - if (ul & (1 << (n - 1))) { + if (num & (1 << (n - 1))) { PCHAR(tmp ? ',' : '<'); for (; (n = *p) > ' '; ++p) PCHAR(n); @@ -611,15 +594,12 @@ } break; case 'd': - if (qflag) - uq = va_arg(ap, quad_t); - else if (lflag) - ul = va_arg(ap, long); - else - ul = va_arg(ap, int); - sign = 1; base = 10; - goto number; + sign = 1; + goto handle_sign; + case 'j': + jflag = 1; + goto reswitch; case 'l': if (lflag) { lflag = 0; @@ -627,34 +607,33 @@ } else lflag = 1; goto reswitch; - case 'o': - if (qflag) - uq = va_arg(ap, u_quad_t); + case 'n': + if (jflag) + *(va_arg(ap, intmax_t *)) = retval; + else if (qflag) + *(va_arg(ap, quad_t *)) = retval; else if (lflag) - ul = va_arg(ap, u_long); + *(va_arg(ap, long *)) = retval; else - ul = va_arg(ap, u_int); + *(va_arg(ap, int *)) = retval; + break; + case 'o': base = 8; - goto nosign; + goto handle_nosign; case 'p': - ul = (uintptr_t)va_arg(ap, void *); base = 16; sharpflag = (width == 0); - goto nosign; + sign = 0; + num = (uintptr_t)va_arg(ap, void *); + goto number; case 'q': qflag = 1; goto reswitch; - case 'n': case 'r': - if (qflag) - uq = va_arg(ap, u_quad_t); - else if (lflag) - ul = va_arg(ap, u_long); - else - ul = sign ? - (u_long)va_arg(ap, int) : va_arg(ap, u_int); base = radix; - goto number; + if (sign) + goto handle_sign; + goto handle_nosign; case 's': p = va_arg(ap, char *); if (p == NULL) @@ -677,50 +656,43 @@ PCHAR(padc); break; case 'u': - if (qflag) - uq = va_arg(ap, u_quad_t); - else if (lflag) - ul = va_arg(ap, u_long); - else - ul = va_arg(ap, u_int); base = 10; - goto nosign; + goto handle_nosign; case 'x': case 'X': - if (qflag) - uq = va_arg(ap, u_quad_t); - else if (lflag) - ul = va_arg(ap, u_long); - else - ul = va_arg(ap, u_int); base = 16; - goto nosign; + goto handle_nosign; case 'z': - if (qflag) - uq = va_arg(ap, u_quad_t); + base = 16; + if (sign) + goto handle_sign; +handle_nosign: + sign = 0; + if (jflag) + num = va_arg(ap, uintmax_t); + else if (qflag) + num = va_arg(ap, u_quad_t); else if (lflag) - ul = va_arg(ap, u_long); + num = va_arg(ap, u_long); else - ul = sign ? - (u_long)va_arg(ap, int) : va_arg(ap, u_int); - base = 16; + num = va_arg(ap, u_int); goto number; -nosign: sign = 0; -number: - if (qflag) { - if (sign && (quad_t)uq < 0) { - neg = 1; - uq = -(quad_t)uq; - } - p = ksprintqn(nbuf, uq, base, &tmp); - } else { - if (sign && (long)ul < 0) { - neg = 1; - ul = -(long)ul; - } - p = ksprintn(nbuf, ul, base, &tmp); +handle_sign: + if (jflag) + num = va_arg(ap, intmax_t); + else if (qflag) + num = va_arg(ap, quad_t); + else if (lflag) + num = va_arg(ap, long); + else + num = va_arg(ap, int); +number: + if (sign && (intmax_t)num < 0) { + neg = 1; + num = -(intmax_t)num; } - if (sharpflag && (qflag ? uq != 0 : ul != 0)) { + p = ksprintn(nbuf, num, base, &tmp); + if (sharpflag && num != 0) { if (base == 8) tmp++; else if (base == 16) @@ -734,7 +706,7 @@ PCHAR(padc); if (neg) PCHAR('-'); - if (sharpflag && (qflag ? uq != 0 : ul != 0)) { + if (sharpflag && num != 0) { if (base == 8) { PCHAR('0'); } else if (base == 16) { @@ -752,10 +724,8 @@ break; default: - PCHAR('%'); - if (lflag) - PCHAR('l'); - PCHAR(ch); + while (percent < fmt) + PCHAR(*percent++); break; } } @@ -783,7 +753,7 @@ dangling = 0; } msgaddchar('<', NULL); - for (p = ksprintn(nbuf, (u_long)pri, 10, NULL); *p;) + for (p = ksprintn(nbuf, (uintmax_t)pri, 10, NULL); *p;) msgaddchar(*p--, NULL); msgaddchar('>', NULL); lastpri = pri; @@ -908,7 +878,7 @@ } SYSCTL_PROC(_kern, OID_AUTO, msgbuf_clear, - CTLTYPE_INT | CTLFLAG_RW | CTLFLAG_SECURE, &msgbuf_clear, 0, + CTLTYPE_INT | CTLFLAG_RW | CTLFLAG_SECURE, &msgbuf_clear, 0, sysctl_kern_msgbuf_clear, "I", "Clear kernel message buffer"); #include "opt_ddb.h" --=-=-=-- To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Sun Jun 2 8:25:45 2002 Delivered-To: freebsd-audit@freebsd.org Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by hub.freebsd.org (Postfix) with ESMTP id D009A37B401 for ; Sun, 2 Jun 2002 08:25:42 -0700 (PDT) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id BAA04772; Mon, 3 Jun 2002 01:25:31 +1000 Date: Mon, 3 Jun 2002 01:29:08 +1000 (EST) From: Bruce Evans X-X-Sender: bde@gamplex.bde.org To: Dag-Erling Smorgrav Cc: Dima Dorfman , Subject: Re: %j for printf(9) In-Reply-To: Message-ID: <20020603012813.M2566-100000@gamplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On 2 Jun 2002, Dag-Erling Smorgrav wrote: > Dag-Erling Smorgrav writes: > > Applied, and renamed fetch_* to handle_*. I'll test it a bit before > > committing. > > Here's the full patch, btw. I'm happy with this version except for a minor style point or two. I'll send a private reply about the style. Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Tue Jun 4 2: 6:40 2002 Delivered-To: freebsd-audit@freebsd.org Received: from mailout07.sul.t-online.com (mailout07.sul.t-online.com [194.25.134.83]) by hub.freebsd.org (Postfix) with ESMTP id 09D7F37B41D for ; Tue, 4 Jun 2002 02:05:43 -0700 (PDT) Received: from fwd01.sul.t-online.de by mailout07.sul.t-online.com with smtp id 17FAG4-0000cK-0B; Tue, 04 Jun 2002 11:05:40 +0200 Received: from Magelan.Leidinger.net (520065502893-0001@[80.131.105.197]) by fmrl01.sul.t-online.com with esmtp id 17FAG2-1nMX0yC; Tue, 4 Jun 2002 11:05:38 +0200 Received: from Leidinger.net (netchild@localhost [127.0.0.1]) by Magelan.Leidinger.net (8.12.3/8.12.3) with ESMTP id g54975wE003913; Tue, 4 Jun 2002 11:07:09 +0200 (CEST) (envelope-from netchild@Leidinger.net) Message-Id: <200206040907.g54975wE003913@Magelan.Leidinger.net> Date: Tue, 4 Jun 2002 11:07:05 +0200 (CEST) From: Alexander Leidinger Subject: [PATCH] making dump EINTR resistant To: audit@freebsd.org Cc: Benjamin Lewis MIME-Version: 1.0 Content-Type: MULTIPART/mixed; BOUNDARY="0-1804289383-1023181632=:572" Content-Transfer-Encoding: BINARY X-Sender: 520065502893-0001@t-dialin.net Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG --0-1804289383-1023181632=:572 Content-Type: TEXT/plain; charset=us-ascii Hi, Benjamin Lewis had some problems with dump, have a look at the -current archives (Subject: "Is anyone else having trouble with dump(8) on -current?", around May 15.). I had such a problem on 4.x (perhaps 4.2 or 4.3) once too (I wasn't able to reproduce it, Benjamin is able to reproduce it on -current) and wrote the attached patch to solve the problem. Since I wrote it for 4.x, it runs fine (actually on a 4.4 system which does daily backups to a spare harddisk). Benjamin uses it also on a daily basis on -current. The attached diff is against -current. Comments? Bye, Alexander. -- The computer revolution is over. The computers won. http://www.Leidinger.net Alexander @ Leidinger.net GPG fingerprint = C518 BC70 E67F 143F BE91 3365 79E2 9C60 B006 3FE7 --0-1804289383-1023181632=:572 Content-Type: TEXT/plain; name="dump.diff" Content-Disposition: attachment; filename="dump.diff" Index: dumprmt.c =================================================================== RCS file: /big/FreeBSD-CVS/src/sbin/dump/dumprmt.c,v retrieving revision 1.17 diff -u -r1.17 dumprmt.c --- dumprmt.c 20 Mar 2002 22:49:39 -0000 1.17 +++ dumprmt.c 27 Mar 2002 15:27:54 -0000 @@ -114,10 +114,13 @@ t.tv_sec = 0; t.tv_usec = 0; if (select(errfd + 1, &r, NULL, NULL, &t)) { - int i; + int i = 0; char buf[2048]; - if ((i = read(errfd, buf, sizeof(buf) - 1)) > 0) { + do { + i = read(errfd, buf, sizeof(buf)); + } while ((i == -1) && (errno == EINTR)); + if (i - 1 > 0) { buf[i] = '\0'; msg("on %s: %s%s", rmtpeer, buf, buf[i - 1] == '\n' ? "" : "\n"); @@ -243,7 +246,9 @@ /* rmtcall() properly sets errno for us on errors. */ return (n); for (i = 0; i < n; i += cc) { - cc = read(rmtape, buf+i, n - i); + do { + cc = read(rmtape, buf+i, n - i); + } while ((cc == -1) && (errno == EINTR)); if (cc <= 0) rmtconnaborted(0); } @@ -361,8 +366,12 @@ rmtgetb(void) { char c; + int ret_val; - if (read(rmtape, &c, 1) != 1) + do { + ret_val = read(rmtape, &c, 1); + } while ((ret_val == -1) && (errno == EINTR)); + if (ret_val != 1) rmtconnaborted(0); return (c); } Index: itime.c =================================================================== RCS file: /big/FreeBSD-CVS/src/sbin/dump/itime.c,v retrieving revision 1.11 diff -u -r1.11 itime.c --- itime.c 20 Mar 2002 22:49:39 -0000 1.11 +++ itime.c 27 Mar 2002 14:26:39 -0000 @@ -74,7 +74,10 @@ { FILE *df; - if ((df = fopen(dumpdates, "r")) == NULL) { + do { + df = fopen(dumpdates, "r"); + } while ((df == NULL) && (errno == EINTR)); + if (df == NULL) { if (errno != ENOENT) { msg("WARNING: cannot read %s: %s\n", dumpdates, strerror(errno)); @@ -84,13 +87,19 @@ * Dumpdates does not exist, make an empty one. */ msg("WARNING: no file `%s', making an empty one\n", dumpdates); - if ((df = fopen(dumpdates, "w")) == NULL) { + do { + df = fopen(dumpdates, "w"); + } while ((df == NULL) && (errno == EINTR)); + if (df == NULL) { msg("WARNING: cannot create %s: %s\n", dumpdates, strerror(errno)); return; } (void) fclose(df); - if ((df = fopen(dumpdates, "r")) == NULL) { + do { + df = fopen(dumpdates, "r"); + } while ((df == NULL) && (errno == EINTR)); + if (df == NULL) { quit("cannot read %s even after creating it: %s\n", dumpdates, strerror(errno)); /* NOTREACHED */ @@ -171,7 +180,10 @@ if(uflag == 0) return; - if ((df = fopen(dumpdates, "r+")) == NULL) + do { + df = fopen(dumpdates, "r+"); + } while ((df == NULL) && (errno == EINTR)); + if (df == NULL) quit("cannot rewrite %s: %s\n", dumpdates, strerror(errno)); fd = fileno(df); (void) flock(fd, LOCK_EX); Index: main.c =================================================================== RCS file: /big/FreeBSD-CVS/src/sbin/dump/main.c,v retrieving revision 1.36 diff -u -r1.36 main.c --- main.c 16 May 2002 04:09:56 -0000 1.36 +++ main.c 4 Jun 2002 08:48:51 -0000 @@ -57,6 +57,7 @@ #include #include +#include #include #include #include @@ -335,7 +336,10 @@ else msgtail("to %s\n", tape); - if ((diskfd = open(disk, O_RDONLY)) < 0) + do { + diskfd = open(disk, O_RDONLY); + } while((diskfd < 0) && (errno == EINTR)); + if (diskfd < 0) err(X_STARTUP, "Cannot open %s", disk); if (fstat(diskfd, &sb) != 0) err(X_STARTUP, "%s: stat", disk); Index: optr.c =================================================================== RCS file: /big/FreeBSD-CVS/src/sbin/dump/optr.c,v retrieving revision 1.20 diff -u -r1.20 optr.c --- optr.c 16 May 2002 04:09:56 -0000 1.20 +++ optr.c 4 Jun 2002 08:48:51 -0000 @@ -82,7 +82,10 @@ int back, errcount; FILE *mytty; - if ((mytty = fopen(_PATH_TTY, "r")) == NULL) + do { + mytty = fopen(_PATH_TTY, "r"); + } while ((mytty == NULL) && (errno == EINTR)); + if (mytty == NULL) quit("fopen on %s fails: %s\n", _PATH_TTY, strerror(errno)); attnmessage = question; timeout = 0; Index: tape.c =================================================================== RCS file: /big/FreeBSD-CVS/src/sbin/dump/tape.c,v retrieving revision 1.18 diff -u -r1.18 tape.c --- tape.c 20 Mar 2002 22:49:39 -0000 1.18 +++ tape.c 4 Jun 2002 08:53:44 -0000 @@ -328,7 +328,7 @@ quit("or use no size estimate at all.\n"); } } - (void) close(slaves[f].fd); + while ((close(slaves[f].fd) < 0) && (errno == EINTR)) /* nop */; } while (wait((int *)NULL) >= 0) /* wait for any signals from slaves */ /* void */; @@ -351,10 +351,10 @@ (void)close(tapefd); return; } - (void) close(tapefd); + while ((close(tapefd) < 0) && (errno == EINTR)) /* nop */; while ((f = open(tape, 0)) < 0) sleep (10); - (void) close(f); + while ((close(f) < 0) && (errno == EINTR)) /* nop */; } void @@ -502,6 +502,7 @@ int childpid; int status; int waitpid; + int tmpfd; char *p; sig_t interrupt_save; @@ -590,12 +591,16 @@ nexttape = NULL; msg("Dumping volume %d on %s\n", tapeno, tape); } + + if (0 != pipeout) + do { + tmpfd = open(tape, O_WRONLY|O_CREAT, 0666); + } while ((tmpfd < 0) && (errno == EINTR)); #ifdef RDUMP while ((tapefd = (host ? rmtopen(tape, 2) : - pipeout ? 1 : open(tape, O_WRONLY|O_CREAT, 0666))) < 0) + pipeout ? 1 : tmpfd)) < 0) #else - while ((tapefd = (pipeout ? 1 : - open(tape, O_WRONLY|O_CREAT, 0666))) < 0) + while ((tapefd = (pipeout ? 1 : tmpfd)) < 0) #endif { msg("Cannot open output \"%s\".\n", tape); @@ -696,7 +701,8 @@ slaves[i].sent = 0; if (slaves[i].pid == 0) { /* Slave starts up here */ for (j = 0; j <= i; j++) - (void) close(slaves[j].fd); + while ((close(slaves[j].fd) < 0) + && (errno == EINTR)) /* nop */; signal(SIGINT, SIG_IGN); /* Master handles this */ doslave(cmd[0], i); Exit(X_FINOK); @@ -739,8 +745,11 @@ /* * Need our own seek pointer. */ - (void) close(diskfd); - if ((diskfd = open(disk, O_RDONLY)) < 0) + while ((close(diskfd) < 0) && (errno == EINTR)) /* nop */; + do { + diskfd = open(disk, O_RDONLY); + } while ((diskfd < 0) && (errno == EINTR)); + if (diskfd < 0) quit("slave couldn't reopen disk: %s\n", strerror(errno)); /* @@ -849,7 +858,13 @@ { int got, need = count; - while ((got = (*func)(fd, buf, need)) > 0 && (need -= got) > 0) - buf += got; + do { + do { + got = (*func)(fd, buf, need); + } while ((got == -1) && (errno == EINTR)); + + if ((got > 0) && ((need -= got) > 0)) + buf += got; + } while (got > 0 && need > 0); return (got < 0 ? got : count - need); } --0-1804289383-1023181632=:572-- To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Tue Jun 4 15:39:21 2002 Delivered-To: freebsd-audit@freebsd.org Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by hub.freebsd.org (Postfix) with ESMTP id 39F9C37B400; Tue, 4 Jun 2002 15:39:17 -0700 (PDT) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id IAA13730; Wed, 5 Jun 2002 08:39:13 +1000 Date: Wed, 5 Jun 2002 08:43:03 +1000 (EST) From: Bruce Evans X-X-Sender: bde@gamplex.bde.org To: Alexander Leidinger Cc: audit@FreeBSD.ORG, Benjamin Lewis Subject: Re: [PATCH] making dump EINTR resistant In-Reply-To: <200206040907.g54975wE003913@Magelan.Leidinger.net> Message-ID: <20020605083335.T5376-100000@gamplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Tue, 4 Jun 2002, Alexander Leidinger wrote: > Benjamin Lewis had some problems with dump, have > a look at the -current archives (Subject: "Is anyone else having trouble > with dump(8) on -current?", around May 15.). > > I had such a problem on 4.x (perhaps 4.2 or 4.3) once too (I wasn't able > to reproduce it, Benjamin is able to reproduce it on -current) and wrote > the attached patch to solve the problem. Since I wrote it for 4.x, it > runs fine (actually on a 4.4 system which does daily backups to a spare > harddisk). Benjamin uses it also on a daily basis on -current. > > The attached diff is against -current. > > Comments? read(), etc., are supposed to be restarted after signals, so the EINTR checks should have no effect in most cases. Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Tue Jun 4 16:47:17 2002 Delivered-To: freebsd-audit@freebsd.org Received: from mail.rpi.edu (mail.rpi.edu [128.113.22.40]) by hub.freebsd.org (Postfix) with ESMTP id 2356A37B406; Tue, 4 Jun 2002 16:47:14 -0700 (PDT) Received: from [128.113.24.47] (gilead.netel.rpi.edu [128.113.24.47]) by mail.rpi.edu (8.12.1/8.12.1) with ESMTP id g54NlBI8570274; Tue, 4 Jun 2002 19:47:12 -0400 Mime-Version: 1.0 X-Sender: drosih@mail.rpi.edu Message-Id: In-Reply-To: <20020605083335.T5376-100000@gamplex.bde.org> References: <20020605083335.T5376-100000@gamplex.bde.org> Date: Tue, 4 Jun 2002 19:47:10 -0400 To: Bruce Evans , Alexander Leidinger From: Garance A Drosihn Subject: Re: [PATCH] making dump EINTR resistant Cc: audit@FreeBSD.ORG Content-Type: text/plain; charset="us-ascii" ; format="flowed" X-Scanned-By: MIMEDefang 2.3 (www dot roaringpenguin dot com slash mimedefang) Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG At 8:43 AM +1000 6/5/02, Bruce Evans wrote: >On Tue, 4 Jun 2002, Alexander Leidinger wrote: >> > > The attached diff is against -current. >> >> Comments? > >read(), etc., are supposed to be restarted after signals, so >the EINTR checks should have no effect in most cases. > Would it be acceptable to add EINTR-type checks to freebsd-ish code? There are some sections of lpr/lpd which do not work right when compiled on other platforms, unless I add EINTR checks at the right places. Admittedly I haven't gone thru and figured out everyplace that should have those checks, I've just added them when I run into some system call which seems to get interrupted a lot. The present lpr code already includes *some* checks for EINTR, but I add a few more (when I compile lpr on other platforms...). Would it be reasonable to add those to the regular freebsd source for lpr? -- Garance Alistair Drosehn = gad@gilead.netel.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 From owner-freebsd-audit Tue Jun 4 18:10:54 2002 Delivered-To: freebsd-audit@freebsd.org Received: from xerxes.courtesan.com (courtesan.com [206.168.103.86]) by hub.freebsd.org (Postfix) with ESMTP id BA57537B411; Tue, 4 Jun 2002 18:10:49 -0700 (PDT) Received: from xerxes.courtesan.com (IDENT:millert@localhost.courtesan.com [127.0.0.1]) by xerxes.courtesan.com (8.12.4/8.12.4) with ESMTP id g551Alpu020379; Tue, 4 Jun 2002 19:10:47 -0600 (MDT) Message-Id: <200206050110.g551Alpu020379@xerxes.courtesan.com> To: Garance A Drosihn Cc: Bruce Evans , Alexander Leidinger , audit@FreeBSD.ORG Subject: Re: [PATCH] making dump EINTR resistant In-reply-to: Your message of "Tue, 04 Jun 2002 19:47:10 EDT." References: <20020605083335.T5376-100000@gamplex.bde.org> Date: Tue, 04 Jun 2002 19:10:47 -0600 From: "Todd C. Miller" Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG In message so spake Garance A Drosihn (drosih): > Would it be acceptable to add EINTR-type checks to freebsd-ish > code? There are some sections of lpr/lpd which do not work > right when compiled on other platforms, unless I add EINTR > checks at the right places. It would probably be better to just make the code in question to just use sigaction() if you are concerned about portability. That way you get consistent handling of syscall restarts. Alternately, you could #define signal to bsd_signal when compiling on non-BSD platforms. That is not quite as portable though. - todd To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Tue Jun 4 19:15:59 2002 Delivered-To: freebsd-audit@freebsd.org Received: from mail.rpi.edu (mail.rpi.edu [128.113.22.40]) by hub.freebsd.org (Postfix) with ESMTP id 7524B37B401; Tue, 4 Jun 2002 19:15:56 -0700 (PDT) Received: from [128.113.24.47] (gilead.netel.rpi.edu [128.113.24.47]) by mail.rpi.edu (8.12.1/8.12.1) with ESMTP id g552FqI8084548; Tue, 4 Jun 2002 22:15:52 -0400 Mime-Version: 1.0 X-Sender: drosih@mail.rpi.edu Message-Id: In-Reply-To: <200206050110.g551Alpu020379@xerxes.courtesan.com> References: <20020605083335.T5376-100000@gamplex.bde.org> <200206050110.g551Alpu020379@xerxes.courtesan.com> Date: Tue, 4 Jun 2002 22:15:51 -0400 To: "Todd C. Miller" From: Garance A Drosihn Subject: Re: [PATCH] making dump EINTR resistant Cc: Bruce Evans , Alexander Leidinger , audit@FreeBSD.ORG Content-Type: text/plain; charset="us-ascii" ; format="flowed" X-Scanned-By: MIMEDefang 2.3 (www dot roaringpenguin dot com slash mimedefang) Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG At 7:10 PM -0600 6/4/02, Todd C. Miller wrote: >In message > so spake Garance A Drosihn (drosih): > >> Would it be acceptable to add EINTR-type checks to freebsd-ish >> code? There are some sections of lpr/lpd which do not work >> right when compiled on other platforms, unless I add EINTR >> checks at the right places. > >It would probably be better to just make the code in question to >just use sigaction() if you are concerned about portability. That >way you get consistent handling of syscall restarts. > >Alternately, you could #define signal to bsd_signal when compiling >on non-BSD platforms. That is not quite as portable though. Ah. Actually I also wrote updates to use sigaction(), but I did that sometime after I had added the EINTR's around various system routines. It sounds like I shouldn't need the EINTR's anymore, but I just didn't realize it. I'll try going that route instead. That sounds much better. Thanks. (I'm still trying to unravel all the various updates I've added to lpr at RPI over the years...) -- Garance Alistair Drosehn = gad@gilead.netel.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 From owner-freebsd-audit Tue Jun 4 19:31:35 2002 Delivered-To: freebsd-audit@freebsd.org Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by hub.freebsd.org (Postfix) with ESMTP id C9B1637B413; Tue, 4 Jun 2002 19:31:24 -0700 (PDT) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id MAA14497; Wed, 5 Jun 2002 12:30:45 +1000 Date: Wed, 5 Jun 2002 12:34:34 +1000 (EST) From: Bruce Evans X-X-Sender: bde@gamplex.bde.org To: "Todd C. Miller" Cc: Garance A Drosihn , Alexander Leidinger , Subject: Re: [PATCH] making dump EINTR resistant In-Reply-To: <200206050110.g551Alpu020379@xerxes.courtesan.com> Message-ID: <20020605121248.U5878-100000@gamplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Tue, 4 Jun 2002, Todd C. Miller wrote: > In message > so spake Garance A Drosihn (drosih): > > > Would it be acceptable to add EINTR-type checks to freebsd-ish > > code? There are some sections of lpr/lpd which do not work > > right when compiled on other platforms, unless I add EINTR > > checks at the right places. This would probably be a lot of work. You would have to check for EINTR after every syscall that may set EINTR and somehow handle this non-error. > It would probably be better to just make the code in question to > just use sigaction() if you are concerned about portability. That > way you get consistent handling of syscall restarts. I agree, but not that SA_RESTART wasn't portable until tomorrow (sic), since it is in POSIX.1-2001 but not in older POSIXes (.1-1996 at least). > Alternately, you could #define signal to bsd_signal when compiling > on non-BSD platforms. That is not quite as portable though. POSIX.1-2001 even has bsd_signal() as standard. BTW: (1) stdio doesn't check for EINTR, so it's not clear how it can work right if en application enables EINTR using siginterrupt(3) or sigaction(2). Working right includes restarting writes from deep in fprintf() just like the kernel would do. (2) Interactive applications that catch signals using non-broken signal handlers ought to enable EINTR so that signals can break them out of blocked read(2)'s, etc. Ones that have had their signal handlers fixed to not do unsafe things tend to get this wrong. E.g., top(1) can't be killed by SIGINT when in command mode (you have to enter a newline to finish the read). Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Wed Jun 5 1:18:40 2002 Delivered-To: freebsd-audit@freebsd.org Received: from mailout09.sul.t-online.com (mailout09.sul.t-online.com [194.25.134.84]) by hub.freebsd.org (Postfix) with ESMTP id 6AFCC37B406; Wed, 5 Jun 2002 01:18:37 -0700 (PDT) Received: from fwd07.sul.t-online.de by mailout09.sul.t-online.com with smtp id 17FW01-0001sJ-05; Wed, 05 Jun 2002 10:18:33 +0200 Received: from Magelan.Leidinger.net (520065502893-0001@[80.131.127.20]) by fmrl07.sul.t-online.com with esmtp id 17FVzr-2CwM40C; Wed, 5 Jun 2002 10:18:23 +0200 Received: from Leidinger.net (netchild@localhost [127.0.0.1]) by Magelan.Leidinger.net (8.12.3/8.12.3) with ESMTP id g558JnPP000786; Wed, 5 Jun 2002 10:19:53 +0200 (CEST) (envelope-from netchild@Leidinger.net) Message-Id: <200206050819.g558JnPP000786@Magelan.Leidinger.net> Date: Wed, 5 Jun 2002 10:19:49 +0200 (CEST) From: Alexander Leidinger Subject: Re: [PATCH] making dump EINTR resistant To: bde@zeta.org.au Cc: netchild@FreeBSD.ORG, audit@FreeBSD.ORG, bhlewis@wossname.net In-Reply-To: <20020605083335.T5376-100000@gamplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/plain; charset=us-ascii X-Sender: 520065502893-0001@t-dialin.net Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On 5 Jun, Bruce Evans wrote: >> Comments? > > read(), etc., are supposed to be restarted after signals, so the EINTR > checks should have no effect in most cases. Yes, I wanted to handle it with sigaction & SA_REASTART, but the APUE book teached me, that signal(3) does restart interrupted syscalls by default on 4.3(+)BSD. Therefore I handled it with do-while code. Stevens lists only ioctl, read, readv, write, writev, wait and waitpid, but not open, fopen, close and fclose. What about those? - I've seen an EINTR related error once on a 4.x system. I can't remember, but I think it was related to an open(2) call. - Benjamin sees such errors on -current, seems there's something broken. Bye, Alexander. -- Actually, Microsoft is sort of a mixture between the Borg and the Ferengi. http://www.Leidinger.net Alexander @ Leidinger.net GPG fingerprint = C518 BC70 E67F 143F BE91 3365 79E2 9C60 B006 3FE7 To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Wed Jun 5 1:24:52 2002 Delivered-To: freebsd-audit@freebsd.org Received: from mailout11.sul.t-online.com (mailout11.sul.t-online.com [194.25.134.85]) by hub.freebsd.org (Postfix) with ESMTP id 9308837B400; Wed, 5 Jun 2002 01:24:48 -0700 (PDT) Received: from fwd08.sul.t-online.de by mailout11.sul.t-online.com with smtp id 17FW61-0002Zb-07; Wed, 05 Jun 2002 10:24:45 +0200 Received: from Magelan.Leidinger.net (520065502893-0001@[80.131.127.20]) by fmrl08.sul.t-online.com with esmtp id 17FW5s-0xww52C; Wed, 5 Jun 2002 10:24:36 +0200 Received: from Leidinger.net (netchild@localhost [127.0.0.1]) by Magelan.Leidinger.net (8.12.3/8.12.3) with ESMTP id g558Q1PP000831; Wed, 5 Jun 2002 10:26:05 +0200 (CEST) (envelope-from netchild@Leidinger.net) Message-Id: <200206050826.g558Q1PP000831@Magelan.Leidinger.net> Date: Wed, 5 Jun 2002 10:26:01 +0200 (CEST) From: Alexander Leidinger Subject: Re: [PATCH] making dump EINTR resistant To: Todd.Miller@courtesan.com Cc: drosih@rpi.edu, bde@zeta.org.au, netchild@FreeBSD.ORG, audit@FreeBSD.ORG In-Reply-To: <200206050110.g551Alpu020379@xerxes.courtesan.com> MIME-Version: 1.0 Content-Type: TEXT/plain; charset=us-ascii X-Sender: 520065502893-0001@t-dialin.net Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On 4 Jun, Todd C. Miller wrote: >> Would it be acceptable to add EINTR-type checks to freebsd-ish >> code? There are some sections of lpr/lpd which do not work >> right when compiled on other platforms, unless I add EINTR >> checks at the right places. > > It would probably be better to just make the code in question to > just use sigaction() if you are concerned about portability. That > way you get consistent handling of syscall restarts. Acording to the APUE book signal(3) is supposed to restart interrupted system calls on 4.3(*)BSD but not on V7, SVR2, SVR3 and SVR4. sigaction with SA_RESTART is supposed to DTRT on 4.3+BSD and SVR4. Stevens lists ioctl, read, readv, write, writev, wait and waitpid as system calls which get automatically restarted. Our man pages also have EINTR for open (+ fopen) and close (+fclose). I've seen an EINTR related error once on a 4.x system, I think it was an EINTR from an open(2) call. Bye, Alexander. -- Failure is not an option. It comes bundled with your Microsoft product. http://www.Leidinger.net Alexander @ Leidinger.net GPG fingerprint = C518 BC70 E67F 143F BE91 3365 79E2 9C60 B006 3FE7 To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Wed Jun 5 1:36:47 2002 Delivered-To: freebsd-audit@freebsd.org Received: from mailout08.sul.t-online.com (mailout08.sul.t-online.com [194.25.134.20]) by hub.freebsd.org (Postfix) with ESMTP id 5743737B406 for ; Wed, 5 Jun 2002 01:36:43 -0700 (PDT) Received: from fwd00.sul.t-online.de by mailout08.sul.t-online.com with smtp id 17FWHZ-00083U-0C; Wed, 05 Jun 2002 10:36:41 +0200 Received: from Magelan.Leidinger.net (520065502893-0001@[80.131.127.20]) by fmrl00.sul.t-online.com with esmtp id 17FWHT-1zzxLsC; Wed, 5 Jun 2002 10:36:35 +0200 Received: from Leidinger.net (netchild@localhost [127.0.0.1]) by Magelan.Leidinger.net (8.12.3/8.12.3) with ESMTP id g558c0PP000854; Wed, 5 Jun 2002 10:38:04 +0200 (CEST) (envelope-from netchild@Leidinger.net) Message-Id: <200206050838.g558c0PP000854@Magelan.Leidinger.net> Date: Wed, 5 Jun 2002 10:38:00 +0200 (CEST) From: Alexander Leidinger Subject: Re: [PATCH] making dump EINTR resistant To: bde@zeta.org.au Cc: Todd.Miller@courtesan.com, drosih@rpi.edu, audit@FreeBSD.ORG In-Reply-To: <20020605121248.U5878-100000@gamplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/plain; charset=us-ascii X-Sender: 520065502893-0001@t-dialin.net Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On 5 Jun, Bruce Evans wrote: >> It would probably be better to just make the code in question to >> just use sigaction() if you are concerned about portability. That >> way you get consistent handling of syscall restarts. > > I agree, but not that SA_RESTART wasn't portable until tomorrow (sic), > since it is in POSIX.1-2001 but not in older POSIXes (.1-1996 at least). Stevens uses this as a portable example for a signal() with restartable syscalls: ---snip--- #include typedef void Sigfunc(int); Sigfunc * signal(int signo, Sigfunc *func) { struct sigaction act, oact; act.sa_handler = func; sigemptyset(&act.sa_mask); act.sa_flags = 0; if (signo == SIGALRM) { #ifdef SA_INTERRUPT act.sa_flags |= SA_INTERRUPT; /* SunOS */ #endif } else { #ifdef SA_RESTART act.sa_flags |= SA_RESTART; /* SVR4, 4.3+BSD */ #endif } if (sigaction(signo, &act, &oact) < 0) return(SIG_ERR); return (oact.sa_handler); } ---snip--- He doesn't want restartable syscalls for SIGALRM to allow to set a time out for I/O operations. Bye, Alexander. -- Give a man a fish and you feed him for a day; teach him to use the Net and he won't bother you for weeks. http://www.Leidinger.net Alexander @ Leidinger.net GPG fingerprint = C518 BC70 E67F 143F BE91 3365 79E2 9C60 B006 3FE7 To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Wed Jun 5 13:39:29 2002 Delivered-To: freebsd-audit@freebsd.org Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by hub.freebsd.org (Postfix) with ESMTP id B404037B40F; Wed, 5 Jun 2002 13:39:25 -0700 (PDT) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id GAA12476; Thu, 6 Jun 2002 06:39:11 +1000 Date: Thu, 6 Jun 2002 06:43:03 +1000 (EST) From: Bruce Evans X-X-Sender: bde@gamplex.bde.org To: Alexander Leidinger Cc: netchild@FreeBSD.ORG, , Subject: Re: [PATCH] making dump EINTR resistant In-Reply-To: <200206050819.g558JnPP000786@Magelan.Leidinger.net> Message-ID: <20020606063157.V8685-100000@gamplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Wed, 5 Jun 2002, Alexander Leidinger wrote: > On 5 Jun, Bruce Evans wrote: > > read(), etc., are supposed to be restarted after signals, so the EINTR > > checks should have no effect in most cases. > > Yes, I wanted to handle it with sigaction & SA_REASTART, but the APUE > book teached me, that signal(3) does restart interrupted syscalls by > default on 4.3(+)BSD. Therefore I handled it with do-while code. > > Stevens lists only ioctl, read, readv, write, writev, wait and waitpid, > but not open, fopen, close and fclose. What about those? Stevens is correct, but in practice open() and close() often ignores signals while it is in the kernel, so signal delivery occurs when the process is about to leave the kernel and doesn't result in the syscall returning EINTR. I think this (no EINTR) happens for regular files and disk devices. It doesn't happen for slow devices like terminals, and shouldn't happen if the syscall can block (then the device driver must support the syscall being aborted by a signal). fopen() and fclose() are based on open() and close(), and hopefully don't make any other syscalls that could affect this. (POSIX more or less specifies no surprises here.) Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Wed Jun 5 13:48:37 2002 Delivered-To: freebsd-audit@freebsd.org Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by hub.freebsd.org (Postfix) with ESMTP id 932FA37B449 for ; Wed, 5 Jun 2002 13:47:52 -0700 (PDT) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id GAA12923; Thu, 6 Jun 2002 06:47:34 +1000 Date: Thu, 6 Jun 2002 06:51:27 +1000 (EST) From: Bruce Evans X-X-Sender: bde@gamplex.bde.org To: Alexander Leidinger Cc: Todd.Miller@courtesan.com, , Subject: Re: [PATCH] making dump EINTR resistant In-Reply-To: <200206050838.g558c0PP000854@Magelan.Leidinger.net> Message-ID: <20020606064418.N8685-100000@gamplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Wed, 5 Jun 2002, Alexander Leidinger wrote: > On 5 Jun, Bruce Evans wrote: > > >> It would probably be better to just make the code in question to > >> just use sigaction() if you are concerned about portability. That > >> way you get consistent handling of syscall restarts. > > > > I agree, but not that SA_RESTART wasn't portable until tomorrow (sic), > > since it is in POSIX.1-2001 but not in older POSIXes (.1-1996 at least). > > Stevens uses this as a portable example for a signal() with restartable > syscalls: > > ---snip--- > #include > > typedef void Sigfunc(int); > > Sigfunc * > signal(int signo, Sigfunc *func) > { > struct sigaction act, oact; > > act.sa_handler = func; > sigemptyset(&act.sa_mask); > act.sa_flags = 0; > if (signo == SIGALRM) { > #ifdef SA_INTERRUPT > act.sa_flags |= SA_INTERRUPT; /* SunOS */ > #endif > } else { > #ifdef SA_RESTART > act.sa_flags |= SA_RESTART; /* SVR4, 4.3+BSD */ > #endif > } > if (sigaction(signo, &act, &oact) < 0) > return(SIG_ERR); > return (oact.sa_handler); > } It's not very portable since it doesn't give restartable syscalls on systems that don't have SA_INTERRUPT or SA_RESTART. The rest of the application would have to deal with this possibilty, and once it does that it can just slightly more easily deal with non-restartable syscalls in all cases. > He doesn't want restartable syscalls for SIGALRM to allow to set a time > out for I/O operations. This is probably necessary for programs that need i/o's to time out or be terminated by a keyboard signal, but it requires the whole program to be concerned about i/o's failing with EINTR -- it is only a small simplification for only one type of signal to not restart syscalls, since that signal may occur. Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Thu Jun 6 3:36:48 2002 Delivered-To: freebsd-audit@freebsd.org Received: from mailout01.sul.t-online.com (mailout01.sul.t-online.com [194.25.134.80]) by hub.freebsd.org (Postfix) with ESMTP id E5D6937B401; Thu, 6 Jun 2002 03:36:44 -0700 (PDT) Received: from fwd09.sul.t-online.de by mailout01.sul.t-online.com with smtp id 17FudF-0002Ma-07; Thu, 06 Jun 2002 12:36:41 +0200 Received: from Magelan.Leidinger.net (520065502893-0001@[217.229.220.203]) by fmrl09.sul.t-online.com with esmtp id 17Fud4-0DaXqqC; Thu, 6 Jun 2002 12:36:30 +0200 Received: from Leidinger.net (netchild@localhost [127.0.0.1]) by Magelan.Leidinger.net (8.12.3/8.12.3) with ESMTP id g56AbviS000861; Thu, 6 Jun 2002 12:38:01 +0200 (CEST) (envelope-from netchild@Leidinger.net) Message-Id: <200206061038.g56AbviS000861@Magelan.Leidinger.net> Date: Thu, 6 Jun 2002 12:37:57 +0200 (CEST) From: Alexander Leidinger Subject: Re: [PATCH] making dump EINTR resistant To: bde@zeta.org.au Cc: netchild@FreeBSD.ORG, audit@FreeBSD.ORG, bhlewis@wossname.net In-Reply-To: <20020606063157.V8685-100000@gamplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/plain; charset=us-ascii X-Sender: 520065502893-0001@t-dialin.net Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On 6 Jun, Bruce Evans wrote: >> Stevens lists only ioctl, read, readv, write, writev, wait and waitpid, >> but not open, fopen, close and fclose. What about those? > > Stevens is correct, but in practice open() and close() often ignores signals > while it is in the kernel, so signal delivery occurs when the process is > about to leave the kernel and doesn't result in the syscall returning > EINTR. I think this (no EINTR) happens for regular files and disk devices. > It doesn't happen for slow devices like terminals, and shouldn't happen if > the syscall can block (then the device driver must support the syscall > being aborted by a signal). So why produced dump a EINTR error for me on 4.x and for Benjamin on -current? Bye, Alexander. -- "One world, one web, one program" -- Microsoft promotional ad "Ein Volk, ein Reich, ein Fuehrer" -- Adolf Hitler http://www.Leidinger.net Alexander @ Leidinger.net GPG fingerprint = C518 BC70 E67F 143F BE91 3365 79E2 9C60 B006 3FE7 To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Sat Jun 8 14:52:34 2002 Delivered-To: freebsd-audit@freebsd.org Received: from mail.rpi.edu (mail.rpi.edu [128.113.22.40]) by hub.freebsd.org (Postfix) with ESMTP id 7E56937B406 for ; Sat, 8 Jun 2002 14:52:10 -0700 (PDT) Received: from [128.113.24.47] (gilead.netel.rpi.edu [128.113.24.47]) by mail.rpi.edu (8.12.1/8.12.1) with ESMTP id g58Lq8vA062086; Sat, 8 Jun 2002 17:52:08 -0400 Mime-Version: 1.0 X-Sender: drosih@mail.rpi.edu Message-Id: Date: Sat, 8 Jun 2002 17:52:07 -0400 To: freebsd-audit@freebsd.org From: Garance A Drosihn Subject: Rewrite of much of lpc/cmds.c ... Cc: freebsd-print@bostonradio.org Content-Type: text/plain; charset="us-ascii" ; format="flowed" X-Scanned-By: MIMEDefang 2.3 (www dot roaringpenguin dot com slash mimedefang) Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG So, as I compare freebsd's lpc to RPI's lpc, I notice two things about many of lpc's commands: 1) a lot of commands duplicate the code to modify the state of a queue. 2) many of them have 'euid' (the priv uid) in effect when they write out any error messages. this seems like a bad idea. That sounds like a tiny thread, but as I pull on it I ended up rewriting more and more of lpc/cmds.c. I figure I better stop at this point, before I rewrite the entire thing. Here is an update which basically rewrites abort disable enable restart start stop up It is also available at: http://people.freebsd.org/~gad/lpr/lpc-cmds.diff This just adds the new implementations. The previous versions remain available as "x"-ed commands (eg: xabort, xdisable, etc). This way, if you think there is something odd about the new implementation, you can resort to the previous implementation and see if it's really any different. A later update will remove the "x"-ed versions. From a user standpoint, this is meant to be an almost invisible change, except that some error situations give a better error message (IMO). One other visible change is that the 'restart' command will only print the printer's name once, instead of printing it once for the 'abort' part, and a second time for the 'restart' part. That had always annoyed me... I would like to commit this to current around June 15th. The two main workhorse routines are set_qstate and kill_qtask. Since this does not delete anything, you'd have to look at the original lpc source to see what the old code did. Here is the update: Index: common_source/common.c =================================================================== RCS file: /home/ncvs/src/usr.sbin/lpr/common_source/common.c,v retrieving revision 1.25 diff -u -r1.25 common.c --- common_source/common.c 28 May 2002 19:21:10 -0000 1.25 +++ common_source/common.c 8 Jun 2002 21:16:56 -0000 @@ -50,6 +50,7 @@ #include #include +#include #include #include #include @@ -250,6 +251,134 @@ snprintf(buf, len, "%s/%s", pp->spool_dir, pp->status_file); return buf; +} + +/* + * Routine to change operational state of a print queue. The operational + * state is indicated by the access bits on the lock file for the queue. + * At present, this is only called from various routines in lpc/cmds.c. + * + * XXX - Note that this works by changing access-bits on the + * file, and you can only do that if you are the owner + * of the file, or root. Thus, this won't really work + * for userids in the "LPR_OPER" group, unless lpc is + * running setuid to root or maybe setuid to daemon. + * By default, lpc is installed setgid to daemon, but + * does not run setuid. + */ +int +set_qstate(int action, const char *lfname) +{ + struct stat stbuf; + mode_t chgbits, newbits, oldmask; + const char *failmsg, *okmsg; + int chres, errsav, fd, res, statres; + + /* + * Find what the current access-bits are. + */ + memset(&stbuf, 0, sizeof(stbuf)); + seteuid(euid); + statres = stat(lfname, &stbuf); + errsav = errno; + seteuid(uid); + if ((statres < 0) && (errsav != ENOENT)) { + printf("\tcannot stat() lock file\n"); + return (SQS_STATFAIL); + /* NOTREACHED */ + } + + /* + * Determine which bit(s) should change for the requested action. + */ + chgbits = stbuf.st_mode; + newbits = LOCK_FILE_MODE; + okmsg = NULL; + failmsg = NULL; + if (action & SQS_DISABLEQ) { + chgbits |= LFM_QUEUE_DIS; + newbits |= LFM_QUEUE_DIS; + okmsg = "queuing disabled"; + failmsg = "disable queuing"; + } + if (action & SQS_STOPP) { + chgbits |= LFM_PRINT_DIS; + newbits |= LFM_PRINT_DIS; + okmsg = "printing disabled"; + failmsg = "disable printing"; + } + if (action & SQS_ENABLEQ) { + chgbits &= ~LFM_QUEUE_DIS; + newbits &= ~LFM_QUEUE_DIS; + okmsg = "queuing enabled"; + failmsg = "enable queuing"; + } + if (action & SQS_STARTP) { + chgbits &= ~LFM_PRINT_DIS; + newbits &= ~LFM_PRINT_DIS; + okmsg = "printing enabled"; + failmsg = "enable printing"; + } + if (okmsg == NULL) { + /* This routine was called with an invalid action. */ + printf("\t\n"); + return (SQS_PARMERR); + /* NOTREACHED */ + } + + res = 0; + if (statres >= 0) { + /* The file already exists, so change the access. */ + seteuid(euid); + chres = chmod(lfname, chgbits); + errsav = errno; + seteuid(uid); + res = SQS_CHGOK; + if (res < 0) + res = SQS_CHGFAIL; + } else if (newbits == LOCK_FILE_MODE) { + /* + * The file does not exist, but the state requested is + * the same as the default state when no file exists. + * Thus, there is no need to create the file. + */ + res = SQS_SKIPCREOK; + } else { + /* + * The file did not exist, so create it with the + * appropriate access bits for the requested action. + * Push a new umask around that create, to make sure + * all the read/write bits are set as desired. + */ + oldmask = umask(S_IWOTH); + seteuid(euid); + fd = open(lfname, O_WRONLY|O_CREAT, newbits); + errsav = errno; + seteuid(uid); + umask(oldmask); + res = SQS_CREFAIL; + if (fd >= 0) { + res = SQS_CREOK; + close(fd); + } + } + + switch (res) { + case SQS_CHGOK: + case SQS_CREOK: + case SQS_SKIPCREOK: + printf("\t%s\n", okmsg); + break; + case SQS_CREFAIL: + printf("\tcannot create lock file: %s\n", + strerror(errsav)); + break; + default: + printf("\tcannot %s: %s\n", failmsg, strerror(errsav)); + break; + } + + return (res); } /* routine to get a current timestamp, optionally in a standard-fmt string */ Index: common_source/lp.h =================================================================== RCS file: /home/ncvs/src/usr.sbin/lpr/common_source/lp.h,v retrieving revision 1.20 diff -u -r1.20 lp.h --- common_source/lp.h 23 Apr 2002 00:06:10 -0000 1.20 +++ common_source/lp.h 8 Jun 2002 21:16:56 -0000 @@ -223,6 +223,23 @@ #define TEMP_FILE_MODE (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH) /* + * Bit-flags for set_qstate() actions, followed by the return values. + */ +#define SQS_DISABLEQ 0x01 /* Disable the queuing of new jobs */ +#define SQS_STOPP 0x02 /* Stop the printing of jobs */ +#define SQS_ENABLEQ 0x10 /* Enable the queuing of new jobs */ +#define SQS_STARTP 0x20 /* Start the printing of jobs */ + +#define SQS_PARMERR -9 /* Invalid parameters from caller */ +#define SQS_CREFAIL -3 /* File did not exist, and create failed */ +#define SQS_CHGFAIL -2 /* File exists, but unable to change state */ +#define SQS_STATFAIL -1 /* Unable to stat() the lock file */ +#define SQS_CHGOK 1 /* File existed, and the state was changed */ +#define SQS_CREOK 2 /* File did not exist, but was created OK */ +#define SQS_SKIPCREOK 3 /* File did not exist, and there was */ + /* no need to create it */ + +/* * Command codes used in the protocol. */ #define CMD_CHECK_QUE '\1' @@ -272,6 +289,7 @@ void rmjob(const char *_printer); void rmremote(const struct printer *_pp); void setprintcap(char *_newfile); +int set_qstate(int _action, const char *_lfname); void show(const char *_nfile, const char *_datafile, int _copies); int startdaemon(const struct printer *_pp); char *status_file_name(const struct printer *_pp, char *_buf, Index: lpc/cmds.c =================================================================== RCS file: /home/ncvs/src/usr.sbin/lpr/lpc/cmds.c,v retrieving revision 1.24 diff -u -r1.24 cmds.c --- lpc/cmds.c 22 Apr 2002 23:28:42 -0000 1.24 +++ lpc/cmds.c 8 Jun 2002 21:16:56 -0000 @@ -70,9 +70,18 @@ #include "extern.h" #include "pathnames.h" +/* + * Return values from kill_qtask(). + */ +#define KQT_LFERROR -2 +#define KQT_KILLFAIL -1 +#define KQT_NODAEMON 0 +#define KQT_KILLOK 1 + static void abortpr(struct printer *_pp, int _dis); static int doarg(char *_job); static int doselect(struct dirent *_d); +static int kill_qtask(const char *lf); static void putmsg(struct printer *_pp, int _argc, char **_argv); static int sortq(const void *_a, const void *_b); static void startpr(struct printer *_pp, int _chgenable); @@ -278,6 +287,140 @@ } /* + * kill an existing daemon and disable printing. + */ +void +abort_q(struct printer *pp) +{ + int killres, setres; + char lf[MAXPATHLEN]; + + lock_file_name(pp, lf, sizeof lf); + printf("%s:\n", pp->printer); + + /* + * Turn on the owner execute bit of the lock file to disable printing. + */ + setres = set_qstate(SQS_STOPP, lf); + + /* + * If set_qstate found that there already was a lock file, then + * call a routine which will read that lock file and kill the + * lpd-process which is listed in that lock file. If the lock + * file did not exist, then either there is no daemon running + * for this queue, or there is one running but *it* could not + * write a lock file (which means we can not determine the + * process id of that lpd-process). + */ + switch (setres) { + case SQS_CHGOK: + case SQS_CHGFAIL: + /* Kill the process */ + killres = kill_qtask(lf); + break; + case SQS_CREOK: + case SQS_CREFAIL: + printf("\tno daemon to abort\n"); + break; + case SQS_STATFAIL: + printf("\tassuming no daemon to abort\n"); + break; + default: + printf("\t\n", + setres); + break; + } + +} + +/* + * Kill the current daemon, to stop printing of the active job. + */ +static int +kill_qtask(const char *lf) +{ + FILE *fp; + pid_t pid; + int errsav, killres, lockres, res; + + seteuid(euid); + fp = fopen(lf, "r"); + errsav = errno; + seteuid(uid); + res = KQT_NODAEMON; + if (fp == NULL) { + /* + * If there is no lock file, then there is no daemon to + * kill. Any other error return means there is some + * kind of problem with the lock file. + */ + if (errsav != ENOENT) + res = KQT_LFERROR; + goto killdone; + } + + /* If the lock file is empty, then there is no daemon to kill */ + if (getline(fp) == 0) + goto killdone; + + /* + * If the file can be locked without blocking, then there + * no daemon to kill, or we should not try to kill it. + * + * XXX - not sure I understand the reasoning behind this... + */ + lockres = flock(fileno(fp), LOCK_SH|LOCK_NB); + (void) fclose(fp); + if (lockres == 0) + goto killdone; + + pid = atoi(line); + if (pid < 0) { + /* + * If we got a negative pid, then the contents of the + * lock file is not valid. + */ + res = KQT_LFERROR; + goto killdone; + } + + seteuid(uid); + killres = kill(pid, SIGTERM); + errsav = errno; + seteuid(uid); + if (killres == 0) { + res = KQT_KILLOK; + printf("\tdaemon (pid %d) killed\n", pid); + } else if (errno == ESRCH) { + res = KQT_NODAEMON; + } else { + res = KQT_KILLFAIL; + printf("\tWarning: daemon (pid %d) not killed:\n", pid); + printf("\t %s\n", strerror(errsav)); + } + +killdone: + switch (res) { + case KQT_LFERROR: + printf("\tcannot open lock file: %s\n", + strerror(errsav)); + break; + case KQT_NODAEMON: + printf("\tno daemon to abort\n"); + break; + case KQT_KILLFAIL: + case KQT_KILLOK: + /* These two already printed messages to the user. */ + break; + default: + printf("\t\n"); + break; + } + + return (res); +} + +/* * Write a message into the status file. */ static void @@ -750,6 +893,21 @@ } /* + * Enable queuing to the printer (allow lpr to add new jobs to the queue). + */ +void +enable_q(struct printer *pp) +{ + int setres; + char lf[MAXPATHLEN]; + + lock_file_name(pp, lf, sizeof lf); + printf("%s:\n", pp->printer); + + setres = set_qstate(SQS_ENABLEQ, lf); +} + +/* * Disable queuing. */ void @@ -786,6 +944,21 @@ } /* + * Disable queuing. + */ +void +disable_q(struct printer *pp) +{ + int setres; + char lf[MAXPATHLEN]; + + lock_file_name(pp, lf, sizeof lf); + printf("%s:\n", pp->printer); + + setres = set_qstate(SQS_DISABLEQ, lf); +} + +/* * Disable queuing and printing and put a message into the status file * (reason for being down). */ @@ -924,6 +1097,37 @@ } /* + * Kill and restart the daemon. + */ +void +restart_q(struct printer *pp) +{ + int killres, setres, startok; + char lf[MAXPATHLEN]; + + lock_file_name(pp, lf, sizeof lf); + printf("%s:\n", pp->printer); + + killres = kill_qtask(lf); + + /* + * XXX - if the kill worked, we should probably sleep for + * a second or so before trying to restart the queue. + */ + + /* make sure the queue is set to print jobs */ + setres = set_qstate(SQS_STARTP, lf); + + seteuid(euid); + startok = startdaemon(pp); + seteuid(uid); + if (!startok) + printf("\tcouldn't restart daemon\n"); + else + printf("\tdaemon restarted\n"); +} + +/* * Enable printing on the specified printer and startup the daemon. */ void @@ -962,6 +1166,30 @@ } /* + * Enable printing on the specified printer and startup the daemon. + */ +void +start_q(struct printer *pp) +{ + int setres, startok; + char lf[MAXPATHLEN]; + + lock_file_name(pp, lf, sizeof lf); + printf("%s:\n", pp->printer); + + setres = set_qstate(SQS_STARTP, lf); + + seteuid(euid); + startok = startdaemon(pp); + seteuid(uid); + if (!startok) + printf("\tcouldn't start daemon\n"); + else + printf("\tdaemon started\n"); + seteuid(uid); +} + +/* * Print the status of the printer queue. */ void @@ -1064,6 +1292,28 @@ seteuid(uid); } +/* + * Stop the specified daemon after completing the current job and disable + * printing. + */ +void +stop_q(struct printer *pp) +{ + int setres; + char lf[MAXPATHLEN]; + + lock_file_name(pp, lf, sizeof lf); + printf("%s:\n", pp->printer); + + setres = set_qstate(SQS_STOPP, lf); + + if (setres >= 0) { + seteuid(euid); + upstat(pp, "printing disabled\n"); + seteuid(uid); + } +} + struct jobqueue **queue; int nitems; time_t mtime; @@ -1236,4 +1486,27 @@ up(struct printer *pp) { startpr(pp, 2); +} + +/* + * Enable both queuing & printing, and start printer (undo `down'). + */ +void +up_q(struct printer *pp) +{ + int setres, startok; + char lf[MAXPATHLEN]; + + lock_file_name(pp, lf, sizeof lf); + printf("%s:\n", pp->printer); + + setres = set_qstate(SQS_ENABLEQ+SQS_STARTP, lf); + + seteuid(euid); + startok = startdaemon(pp); + seteuid(uid); + if (!startok) + printf("\tcouldn't start daemon\n"); + else + printf("\tdaemon started\n"); } Index: lpc/cmdtab.c =================================================================== RCS file: /home/ncvs/src/usr.sbin/lpr/lpc/cmdtab.c,v retrieving revision 1.4 diff -u -r1.4 cmdtab.c --- lpc/cmdtab.c 25 Jun 2001 02:05:03 -0000 1.4 +++ lpc/cmdtab.c 8 Jun 2002 21:16:56 -0000 @@ -65,21 +65,28 @@ #define PR 1 /* a privileged command */ struct cmd cmdtab[] = { - { "abort", aborthelp, PR, 0, doabort }, + { "abort", aborthelp, PR, 0, abort_q }, { "clean", cleanhelp, PR, init_clean, clean_q }, - { "enable", enablehelp, PR, 0, enable }, + { "enable", enablehelp, PR, 0, enable_q }, { "exit", quithelp, 0, quit, 0 }, - { "disable", disablehelp, PR, 0, disable }, + { "disable", disablehelp, PR, 0, disable_q }, { "down", downhelp, PR, down, 0 }, { "help", helphelp, 0, help, 0 }, { "quit", quithelp, 0, quit, 0 }, - { "restart", restarthelp, 0, 0, restart }, - { "start", starthelp, PR, 0, startcmd }, + { "restart", restarthelp, 0, 0, restart_q }, + { "start", starthelp, PR, 0, start_q }, { "status", statushelp, 0, 0, status }, - { "stop", stophelp, PR, 0, stop }, + { "stop", stophelp, PR, 0, stop_q }, { "tclean", tcleanhelp, 0, init_tclean, clean_q }, { "topq", topqhelp, PR, topq, 0 }, - { "up", uphelp, PR, 0, up }, + { "up", uphelp, PR, 0, up_q }, + { "xabort", aborthelp, PR, 0, doabort }, + { "xenable", enablehelp, PR, 0, enable }, + { "xdisable", disablehelp, PR, 0, disable }, + { "xrestart", restarthelp, 0, 0, restart }, + { "xstart", starthelp, PR, 0, startcmd }, + { "xstop", stophelp, PR, 0, stop }, + { "xup", uphelp, PR, 0, up }, { "?", helphelp, 0, help, 0 }, { 0, 0, 0, 0, 0}, }; Index: lpc/extern.h =================================================================== RCS file: /home/ncvs/src/usr.sbin/lpr/lpc/extern.h,v retrieving revision 1.5 diff -u -r1.5 extern.h --- lpc/extern.h 25 Jun 2001 02:05:03 -0000 1.5 +++ lpc/extern.h 8 Jun 2002 21:16:56 -0000 @@ -42,11 +42,14 @@ __BEGIN_DECLS +void abort_q(struct printer *_pp); void clean_q(struct printer *_pp); void disable(struct printer *_pp); +void disable_q(struct printer *_pp); void doabort(struct printer *_pp); void down(int _argc, char *_argv[]); void enable(struct printer *_pp); +void enable_q(struct printer *_pp); void generic(void (*_specificrtn)(struct printer *_pp), void (*_initcmd)(int _argc, char *_argv[]), int _argc, char *_argv[]); @@ -55,11 +58,15 @@ void init_tclean(int _argc, char *_argv[]); void quit(int _argc, char *_argv[]); void restart(struct printer *_pp); +void restart_q(struct printer *_pp); void startcmd(struct printer *_pp); +void start_q(struct printer *_pp); void status(struct printer *_pp); void stop(struct printer *_pp); +void stop_q(struct printer *_pp); void topq(int _argc, char *_argv[]); void up(struct printer *_pp); +void up_q(struct printer *_pp); __END_DECLS extern int NCMDS; -- Garance Alistair Drosehn = gad@gilead.netel.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 From owner-freebsd-audit Sat Jun 8 18:14:32 2002 Delivered-To: freebsd-audit@freebsd.org Received: from mailsrv.otenet.gr (mailsrv.otenet.gr [195.170.0.5]) by hub.freebsd.org (Postfix) with ESMTP id 9130837B400 for ; Sat, 8 Jun 2002 18:14:27 -0700 (PDT) Received: from hades.hell.gr (patr530-a091.otenet.gr [212.205.215.91]) by mailsrv.otenet.gr (8.12.3/8.12.3) with ESMTP id g591ENZN028615 for ; Sun, 9 Jun 2002 04:14:25 +0300 (EEST) Received: from hades.hell.gr (hades [127.0.0.1]) by hades.hell.gr (8.12.3/8.12.3) with ESMTP id g591EMUP070546 for ; Sun, 9 Jun 2002 04:14:22 +0300 (EEST) (envelope-from keramida@FreeBSD.org) Received: (from charon@localhost) by hades.hell.gr (8.12.3/8.12.3/Submit) id g591EKuI070545 for audit@FreeBSD.org; Sun, 9 Jun 2002 04:14:21 +0300 (EEST) (envelope-from keramida@FreeBSD.org) Date: Sun, 9 Jun 2002 04:14:19 +0300 From: Giorgos Keramidas To: audit@FreeBSD.org Subject: badsect warns Message-ID: <20020609011418.GA70449@hades.hell.gr> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG The following enables compiling badsect with WARNS=6. There seems to be something funny about fs->fs_size though. In it's not daddr_t but `long'. This is why it triggers the warning now that the cast to `unsigned' (which is there since revision 1.1 of badsect.c) doesn't match the signed type of fs->fs_size. Is there some reason that this is cast to `unsigned'? If not, should we remove the cast and start building badsect with something more strict than WARNS=0? - Giorgos %%% Index: Makefile =================================================================== RCS file: /home/ncvs/src/sbin/badsect/Makefile,v retrieving revision 1.5 diff -u -r1.5 Makefile --- Makefile 4 Dec 2001 02:19:44 -0000 1.5 +++ Makefile 8 Jun 2002 23:55:54 -0000 @@ -2,7 +2,7 @@ # $FreeBSD: src/sbin/badsect/Makefile,v 1.5 2001/12/04 02:19:44 obrien Exp $ PROG= badsect -WARNS= 0 +WARNS= 6 MAN= badsect.8 .include Index: badsect.c =================================================================== RCS file: /home/ncvs/src/sbin/badsect/badsect.c,v retrieving revision 1.13 diff -u -r1.13 badsect.c --- badsect.c 16 May 2002 04:09:51 -0000 1.13 +++ badsect.c 9 Jun 2002 01:00:54 -0000 @@ -168,7 +168,7 @@ daddr_t fsbn, bn; fsbn = dbtofsb(fs, blkno); - if ((unsigned)(fsbn+cnt) > fs->fs_size) { + if ((fsbn + cnt) > fs->fs_size) { printf("block %ld out of range of filesystem\n", (long)blkno); return (1); } %%% To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Sat Jun 8 18:40: 5 2002 Delivered-To: freebsd-audit@freebsd.org Received: from mailsrv.otenet.gr (mailsrv.otenet.gr [195.170.0.5]) by hub.freebsd.org (Postfix) with ESMTP id A75B837B401 for ; Sat, 8 Jun 2002 18:39:58 -0700 (PDT) Received: from hades.hell.gr (patr530-a091.otenet.gr [212.205.215.91]) by mailsrv.otenet.gr (8.12.3/8.12.3) with ESMTP id g591duZN013615 for ; Sun, 9 Jun 2002 04:39:56 +0300 (EEST) Received: from hades.hell.gr (hades [127.0.0.1]) by hades.hell.gr (8.12.3/8.12.3) with ESMTP id g591dtUP072034 for ; Sun, 9 Jun 2002 04:39:55 +0300 (EEST) (envelope-from keramida@FreeBSD.org) Received: (from charon@localhost) by hades.hell.gr (8.12.3/8.12.3/Submit) id g591dsPg072033 for audit@FreeBSD.org; Sun, 9 Jun 2002 04:39:54 +0300 (EEST) (envelope-from keramida@FreeBSD.org) Date: Sun, 9 Jun 2002 04:39:53 +0300 From: Giorgos Keramidas To: audit@FreeBSD.org Subject: warns fixes for bin/date Message-ID: <20020609013953.GA71960@hades.hell.gr> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG The type of a variable passed to getsockopt and recvfrom in src/bin/date/netdate.c is `int'. Changing it to `socklen_t' fixes the only warnings that this utility has when compiling with WARNS=6. How does this look? %%% Index: Makefile =================================================================== RCS file: /home/ncvs/src/bin/date/Makefile,v retrieving revision 1.10 diff -u -r1.10 Makefile --- Makefile 4 Feb 2002 02:49:18 -0000 1.10 +++ Makefile 9 Jun 2002 01:34:21 -0000 @@ -3,7 +3,7 @@ PROG= date SRCS= date.c netdate.c vary.c -WARNS= 0 +WARNS= 6 WFORMAT=0 DPADD= ${LIBUTIL} LDADD= -lutil Index: netdate.c =================================================================== RCS file: /home/ncvs/src/bin/date/netdate.c,v retrieving revision 1.14 diff -u -r1.14 netdate.c --- netdate.c 22 Feb 2002 20:47:20 -0000 1.14 +++ netdate.c 9 Jun 2002 01:34:16 -0000 @@ -76,7 +76,8 @@ struct sockaddr_in lsin, dest, from; fd_set ready; long waittime; - int s, length, port, timed_ack, found, lerr; + int s, port, timed_ack, found, lerr; + socklen_t length; char hostname[MAXHOSTNAMELEN]; if ((sp = getservbyname("timed", "udp")) == NULL) { %%% To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Sat Jun 8 18:45:20 2002 Delivered-To: freebsd-audit@freebsd.org Received: from mailsrv.otenet.gr (mailsrv.otenet.gr [195.170.0.5]) by hub.freebsd.org (Postfix) with ESMTP id 77A7E37B401 for ; Sat, 8 Jun 2002 18:45:17 -0700 (PDT) Received: from hades.hell.gr (patr530-a091.otenet.gr [212.205.215.91]) by mailsrv.otenet.gr (8.12.3/8.12.3) with ESMTP id g591jFZN016734 for ; Sun, 9 Jun 2002 04:45:15 +0300 (EEST) Received: from hades.hell.gr (hades [127.0.0.1]) by hades.hell.gr (8.12.3/8.12.3) with ESMTP id g591jDUP072900 for ; Sun, 9 Jun 2002 04:45:14 +0300 (EEST) (envelope-from keramida@FreeBSD.org) Received: (from charon@localhost) by hades.hell.gr (8.12.3/8.12.3/Submit) id g591jDcR072892 for audit@FreeBSD.org; Sun, 9 Jun 2002 04:45:13 +0300 (EEST) (envelope-from keramida@FreeBSD.org) Date: Sun, 9 Jun 2002 04:45:12 +0300 From: Giorgos Keramidas To: audit@FreeBSD.org Subject: minor header change in src/bin/dd/misc.c Message-ID: <20020609014512.GA72307@hades.hell.gr> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG The source of bin/dd/misc.c doesn't use any of the functions of but uses strlen, which is defined in . The following seems to fix the only warning this file triggers for me. Does it seem ok to you all? %%% Index: misc.c =================================================================== RCS file: /home/ncvs/src/bin/dd/misc.c,v retrieving revision 1.23 diff -u -r1.23 misc.c --- misc.c 2 Feb 2002 06:24:12 -0000 1.23 +++ misc.c 9 Jun 2002 01:42:12 -0000 @@ -49,7 +49,7 @@ #include #include #include -#include +#include #include #include "dd.h" %%% To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Sat Jun 8 18:56:33 2002 Delivered-To: freebsd-audit@freebsd.org Received: from espresso.q9media.com (espresso.q9media.com [216.254.138.122]) by hub.freebsd.org (Postfix) with ESMTP id BCCD137B403; Sat, 8 Jun 2002 18:56:29 -0700 (PDT) Received: (from mike@localhost) by espresso.q9media.com (8.11.6/8.11.6) id g591sRq16014; Sat, 8 Jun 2002 21:54:27 -0400 (EDT) (envelope-from mike) Date: Sat, 8 Jun 2002 21:54:27 -0400 From: Mike Barcroft To: Giorgos Keramidas Cc: audit@FreeBSD.org Subject: Re: warns fixes for bin/date Message-ID: <20020608215427.C87326@espresso.q9media.com> References: <20020609013953.GA71960@hades.hell.gr> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20020609013953.GA71960@hades.hell.gr>; from keramida@FreeBSD.org on Sun, Jun 09, 2002 at 04:39:53AM +0300 Organization: The FreeBSD Project Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Giorgos Keramidas writes: > The type of a variable passed to getsockopt and recvfrom in > src/bin/date/netdate.c is `int'. Changing it to `socklen_t' > fixes the only warnings that this utility has when compiling > with WARNS=6. > > How does this look? I think you might get additional warnings on LP64 system like alpha and sparc64, since `size_t' != `socklen_t' on those systems. It's probably okay to explicitly cast the return values from sizeof() to `socklen_t' for all the uses of `length'. Best regards, Mike Barcroft To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Sat Jun 8 19: 2:34 2002 Delivered-To: freebsd-audit@freebsd.org Received: from espresso.q9media.com (espresso.q9media.com [216.254.138.122]) by hub.freebsd.org (Postfix) with ESMTP id E1DD237B401; Sat, 8 Jun 2002 19:02:31 -0700 (PDT) Received: (from mike@localhost) by espresso.q9media.com (8.11.6/8.11.6) id g5920UE16403; Sat, 8 Jun 2002 22:00:30 -0400 (EDT) (envelope-from mike) Date: Sat, 8 Jun 2002 22:00:30 -0400 From: Mike Barcroft To: Giorgos Keramidas Cc: audit@FreeBSD.org Subject: Re: minor header change in src/bin/dd/misc.c Message-ID: <20020608220030.D87326@espresso.q9media.com> References: <20020609014512.GA72307@hades.hell.gr> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20020609014512.GA72307@hades.hell.gr>; from keramida@FreeBSD.org on Sun, Jun 09, 2002 at 04:45:12AM +0300 Organization: The FreeBSD Project Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Giorgos Keramidas writes: > The source of bin/dd/misc.c doesn't use any of the functions of > but uses strlen, which is defined in . The > following seems to fix the only warning this file triggers for me. > Does it seem ok to you all? Yes, it's correct. is almost always wrong for BSD code. I removed the include of from when I cleaned up those headers for POSIX compatibility, so there are probably other mis-includes throughout the system. Best regards, Mike Barcroft To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Sat Jun 8 19:33:10 2002 Delivered-To: freebsd-audit@freebsd.org Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by hub.freebsd.org (Postfix) with ESMTP id 5E2CC37B405; Sat, 8 Jun 2002 19:33:05 -0700 (PDT) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id MAA29466; Sun, 9 Jun 2002 12:33:02 +1000 Date: Sun, 9 Jun 2002 12:33:47 +1000 (EST) From: Bruce Evans X-X-Sender: bde@gamplex.bde.org To: Giorgos Keramidas Cc: audit@FreeBSD.ORG Subject: Re: badsect warns In-Reply-To: <20020609011418.GA70449@hades.hell.gr> Message-ID: <20020609123105.X21758-100000@gamplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Sun, 9 Jun 2002, Giorgos Keramidas wrote: > The following enables compiling badsect with WARNS=6. > There seems to be something funny about fs->fs_size though. > In it's not daddr_t but `long'. It is actually int32_t, although it is logically a type a tiny bit larger than ufs_daddr_t (ffs block numbers have type ufs_daddr_t and the filesystem size is 1 larger than the largest ffs block number). I think gcc is just warning about a sign mismatch now, but the cast is almost necessary for correct code on machines with >= 32-bit unsigned's. It "fixes" overflow of the sum. fs->fs_size has type int64_t in ufs2, so the cast is just broken except on machines with >= 64-bit unsigneds. ufs2 has similarly broken casts internally, e.g., (u_int)bno in ffs_alloc.c. > Is there some reason that this is cast to `unsigned'? > %%% > Index: badsect.c > =================================================================== > RCS file: /home/ncvs/src/sbin/badsect/badsect.c,v > retrieving revision 1.13 > diff -u -r1.13 badsect.c > --- badsect.c 16 May 2002 04:09:51 -0000 1.13 > +++ badsect.c 9 Jun 2002 01:00:54 -0000 > @@ -168,7 +168,7 @@ > daddr_t fsbn, bn; > > fsbn = dbtofsb(fs, blkno); > - if ((unsigned)(fsbn+cnt) > fs->fs_size) { > + if ((fsbn + cnt) > fs->fs_size) { > printf("block %ld out of range of filesystem\n", (long)blkno); > return (1); > } > %%% The cast seems to be just the usual obsolete hack to check that a number is >= 0 and <= a maximum value unsigned only one comparison: int foo, max: ... if ((u_int)foo > max) err(...); This is obsolete because non-primitive compilers will optimize the obvious check: if (foo < 0 || foo > max) into the above if that is an optimization. It fails to do this correctly in lots of ways: 1) The unsigned cast must be to the same or a wider type than the left operand. 2) gcc tends to warn if the type of thr right operand is signed. 3) fsbn+cnt may have overflowed before we checked it. Casting the terms of the sum would work better -- it prevents overflow provided the terms are nonnegative. However, the terms are not known to be nonnegative here because the caller has not checked its (user-supplied!) input -- `cnt' is always 1, but fsbn is dbtofsb(fs, blkno) which is garbage if blkno is garbage, and blkno may be garbage since it is just atol(*argv). This leads to bugs in the caller: (4) Disk block numbers are read in using atol(), but this can only work if daddr_t has type long. This was broken on machines with 64-bit longs, and ufs2 breaks it on machines with 32-bit longs. (5) There is no overflow checking for atol() of course. (6) Negative daddr_t's are errors in this context (probably in all contexts). They are not checked for, but the checking in chkuse() depends on them being weeded out earlier. 64-bit daddr_t's also break all the casts to long in error messages. 64-bit daddr_t's can't be printed as longs if longs are 32 bits. > If not, should we remove the cast and start building badsect with > something more strict than WARNS=0? Needs more extensive cleanups. Warnings about sign mismatches basically just acientally tell you about overlow of the top bit here. Changing daddr_t to 64 bits tends to cause overflow in 32 other bits. Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message