Date: Thu, 13 Oct 2011 15:48:42 +0200 From: Pawel Jakub Dawidek <pjd@FreeBSD.org> To: Dag-Erling =?iso-8859-1?Q?Sm=F8rgrav?= <des@des.no> Cc: current@freebsd.org Subject: Re: incorrect use of pidfile(3) Message-ID: <20111013134841.GF1667@garage.freebsd.pl> In-Reply-To: <864nzdaw7b.fsf@ds4.des.no> References: <86pqi1b1qp.fsf@ds4.des.no> <864nzdaw7b.fsf@ds4.des.no>
next in thread | previous in thread | raw e-mail | index | archive | help
--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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111013134841.GF1667>