Re: incorrect use of pidfile(3)

From: Pawel Jakub Dawidek <pjd_at_FreeBSD.org>
Date: Fri, 14 Oct 2011 12:05:22 +0200
On Thu, Oct 13, 2011 at 04:11:40PM +0200, Dag-Erling Smørgrav wrote:
> Pawel Jakub Dawidek <pjd_at_FreeBSD.org> writes:
> > 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.
> 
> The historical (and documented) behavior is to return EAGAIN.

We don't want to duplicate the code of handling EAGAIN into every single
pidfile(3) consumer. This is why we hav pidfile(3) API in the first
place - to make it easy for people to use.

> > Also if we now have for loop, why not to put count in there?
> 
> Because if we do, there will be a nanosleep after the last
> pidfile_read() attempt.  We need to break the loop after pidfile_read()
> failed but before nanosleep().

Right, ok.

> > I'm not very happy about touching pidptr in case of error other than
> > EEXIST. This is not documented, but a bit unexpected anyway.
> 
> Well, it was your idea, I just moved it to before the loop :)

In my patch *pidptr was set to -1 only in the case of EAGAIN from
pidfile_read(), not for every other error.

BTW. With your patch we will continue even when flopen(3) failed for
other reasons, instead of returning NULL. Checking for fd being -1
should not be done in the same statement with other checks.

After proposed changes it would look like this, what do you think?

	http://people.freebsd.org/~pjd/patches/pidfile.3.patch

-- 
Pawel Jakub Dawidek                       http://www.wheelsystems.com
FreeBSD committer                         http://www.FreeBSD.org
Am I Evil? Yes, I Am!                     http://yomoli.com

Received on Fri Oct 14 2011 - 08:06:03 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:19 UTC