From owner-freebsd-current@FreeBSD.ORG Thu Oct 13 13:49:30 2011 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 47A481065695 for ; Thu, 13 Oct 2011 13:49:30 +0000 (UTC) (envelope-from pawel@dawidek.net) Received: from mail.dawidek.net (60.wheelsystems.com [83.12.187.60]) by mx1.freebsd.org (Postfix) with ESMTP id 583258FC08 for ; Thu, 13 Oct 2011 13:49:29 +0000 (UTC) Received: from localhost (58.wheelsystems.com [83.12.187.58]) by mail.dawidek.net (Postfix) with ESMTPSA id CED2FD99; Thu, 13 Oct 2011 15:49:24 +0200 (CEST) Date: Thu, 13 Oct 2011 15:48:42 +0200 From: Pawel Jakub Dawidek To: Dag-Erling =?iso-8859-1?Q?Sm=F8rgrav?= Message-ID: <20111013134841.GF1667@garage.freebsd.pl> References: <86pqi1b1qp.fsf@ds4.des.no> <864nzdaw7b.fsf@ds4.des.no> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="GdbWtwDHkcXqP16f" Content-Disposition: inline In-Reply-To: <864nzdaw7b.fsf@ds4.des.no> X-OS: FreeBSD 9.0-CURRENT amd64 User-Agent: Mutt/1.5.21 (2010-09-15) Cc: current@freebsd.org Subject: Re: incorrect use of pidfile(3) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 13 Oct 2011 13:49:30 -0000 --GdbWtwDHkcXqP16f Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Oct 13, 2011 at 02:54:16PM +0200, Dag-Erling Sm=F8rgrav wrote: > After discussing this with pjd@ on IRC, I arrived at the attached patch, > which increases the length of time pidfile_open() itself waits (I hadn't > noticed that it already looped) and sets *pidptr to -1 if it fails to read > a pid. I'm still in opinion that EWOULDBLOCK and EAGAIN (which is the same value on FreeBSD) should be converted to EEXIST on pidfile_open() return. Also if we now have for loop, why not to put count in there? I'm not very happy about touching pidptr in case of error other than EEXIST. This is not documented, but a bit unexpected anyway. I'm happy with increasing the timeout. > Index: lib/libutil/pidfile.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- lib/libutil/pidfile.c (revision 226271) > +++ lib/libutil/pidfile.c (working copy) > @@ -118,22 +118,19 @@ > */ > fd =3D flopen(pfh->pf_path, > O_WRONLY | O_CREAT | O_TRUNC | O_NONBLOCK, mode); > - if (fd =3D=3D -1) { > - count =3D 0; > + if (fd =3D=3D -1 && errno =3D=3D EWOULDBLOCK && pidptr !=3D NULL) { > + *pidptr =3D -1; > + count =3D 20; > rqtp.tv_sec =3D 0; > rqtp.tv_nsec =3D 5000000; > - if (errno =3D=3D EWOULDBLOCK && pidptr !=3D NULL) { > - again: > + for (;;) { > errno =3D pidfile_read(pfh->pf_path, pidptr); > - if (errno =3D=3D 0) > - errno =3D EEXIST; > - else if (errno =3D=3D EAGAIN) { > - if (++count <=3D 3) { > - nanosleep(&rqtp, 0); > - goto again; > - } > - } > + if (errno !=3D EAGAIN || --count =3D=3D 0) > + break; > + nanosleep(&rqtp, 0); > } > + if (errno =3D=3D 0) > + errno =3D EEXIST; > free(pfh); > return (NULL); > } --=20 Pawel Jakub Dawidek http://www.wheelsystems.com FreeBSD committer http://www.FreeBSD.org Am I Evil? Yes, I Am! http://yomoli.com --GdbWtwDHkcXqP16f Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.14 (FreeBSD) iEYEARECAAYFAk6W7DkACgkQForvXbEpPzRUbQCeJlsMsNYJvPMRKe4Ht9c82fZx vLkAoMzzW86nyjmzY7QLmwzMxHOFf7lQ =ngQc -----END PGP SIGNATURE----- --GdbWtwDHkcXqP16f--