Date: Wed, 19 Sep 2007 01:12:12 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Joe Marcus Clarke <marcus@FreeBSD.org> Cc: Jilles Tjoelker <jilles@stack.nl>, freebsd-arch@FreeBSD.org Subject: Re: Understanding interrupted system calls Message-ID: <20070919005425.P75789@besplex.bde.org> In-Reply-To: <1189629164.80084.81.camel@shumai.marcuscom.com> References: <1188600721.1255.11.camel@shumai.marcuscom.com> <20070901112600.GA33832@stack.nl> <1188660782.41727.5.camel@shumai.marcuscom.com> <20070901224025.GA97796@stack.nl> <20070902131910.H46281@delplex.bde.org> <1189629164.80084.81.camel@shumai.marcuscom.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 12 Sep 2007, Joe Marcus Clarke wrote: Sorry this reply took so long. > On Sun, 2007-09-02 at 14:17 +1000, Bruce Evans wrote: >>> From Jilles' previous reply: >>>>> The fixed version would then be >>>>> >>>>> error = tsleep(&scp->smode, PZERO|PCATCH, "waitvt", 0); >> >> I think this is right. The kernel should never loop on ERESTART like this. >> Please fix the remaining style bug in it (missing spaces around binary >> operator). > > Bruce, I didn't know if you saw my fix, if it needs more work, or if > you're going to commit it? I'd really like to get this fixed before > 7.0. Should I open a PR to track it? Thanks. > > http://www.marcuscom.com/downloads/syscons.c.diff > http://www.marcuscom.com/downloads/pcvt_ext.c (-STABLE only) I saw a new style bug in it, but wasn't going to complain. Now I will :-). % --- src/sys/dev/syscons/syscons.c.orig 2007-09-02 23:04:15.000000000 -0400 % +++ src/sys/dev/syscons/syscons.c 2007-09-02 23:05:06.000000000 -0400 % @@ -1073,8 +1073,7 @@ scioctl(struct cdev *dev, u_long cmd, ca % scp = sc_get_stat(SC_DEV(sc, i)); % if (scp == scp->sc->cur_scp) % return 0; % - while ((error=tsleep(&scp->smode, PZERO|PCATCH, % - "waitvt", 0)) == ERESTART) ; % + error = tsleep(&scp->smode, (PZERO|PCATCH), "waitvt", 0); % return error; % % case VT_GETACTIVE: /* get active vty # */ It now has extra parentheses, and still doesn't have spaces around the binary operator. In pcvt_ext.c.diff, the changes are larger and I only glanced at most of them, but noticed that the formatting of the `PZERO | PCATCH' are was already what I want but was changed to the above (2 changes instead of 1). pcvt is almost dead and its style so unusual that it is difficult to be bug for bug compatible with, so I would try to avoid making any style changes in it. Please commit all the changes, preferably after fixing the style bugs. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070919005425.P75789>