Subject: Re: pkg/19002: rc.d/cyrus slightly broken
To: NetBSD GNATS submissions and followups <gnats-bugs@gnats.netbsd.org>
From: Chris Gilbert <chris@dokein.co.uk>
List: netbsd-bugs
Date: 11/10/2002 22:35:01
Which Cyrus is this btw?  I'm assuming it's 2.1.9...

On Sun, 10 Nov 2002 11:48:45 -0500 (EST)
woods@weird.com (Greg A. Woods) wrote:

> [ On Sunday, November 10, 2002 at 02:48:42 (-0800), Frank Cusack
> wrote: ]
> > Subject: pkg/19002: rc.d/cyrus slightly broken
> >
> > 	1) stdin/stdout/stderr should be redirected to /dev/null.  I
> > 	   won't go into why this should be, I'm sure it's well
> > 	   understood.
> 
> In general I very strongly disagree with forcing stdio to /dev/null on
> a daemon on startup.  Properly coded daemons do the right thing at the
> right time and their output should not ever be forcibly discarded on
> their behalf.
>
> What _specific_ reasons do you feel they should be re-directed for
> this particular program?

Certainly cyrus 2.1.9 does this already, from main() in master/master.c:
    if (close_std) {
      /* close stdin/out/err */
      for (fd = 0; fd < 3; fd++) {
	close(fd);
	if (open("/dev/null", O_RDWR, 0) != fd)
	  fatal("couldn't open /dev/null: %m", 2);
      }
    }

where close_std is based on the option -D (with it set close_std == 0)

> > 	2) The 'sleep 2' bit causes master to receive SIGHUP after 2s.
> 
> That sounds like a fairly important bug that should be fixed properly,
> whatever that means.
> 
> I'm not sure exactly where the bug is though.  Does the SIGHUP cause a
> problem for the daemon?  If so, why?  Daemons normally reload their
> configurations on SIGHUP -- does this one do that?
> 
> Are you certain you have correctly determined the cause of the SIGHUP?
> I would have expected that it is normally be generated when /etc/rc
> exits, not when the sleep exits.
> > 	   It doesn't seem to have any real function, but master
> > 	   receiving an unnecessary SIGHUP *is* bad, so removing it
> > 	   seems to be the easiest fix.
> 
> That sleep must have been put there for some reason that the person
> writing it thought was important (no other script has a similar
> feature).  Perhaps we need to ask what that reason is first!

I'd suspect the sleep is there to give the system time to start up
cyrus, without being overloaded by other things.  The & sleep 2 is a
standard trick, see rc.d/xfs.
 
> At first glance it looks like this particular daemon is not designed
> to run stand-alone from /etc/rc on a *BSD system.  If so then it needs
> some(relatively minor) patches to correct this deficiency before it is
> truly viable for use on NetBSD.

It what way does it not fit with rc.d?  I've had it happily running from
rc.d for months on my mail server.

I don't claim it's the perfect package (far from it looking at PR's 8)
but it works (for a limited definition of works 8)  I created the
package so those without time to figure out lots of the package systems
infrastructure but have a requirement for the latest cyrus could stand a
chance of getting it working.

Cheers,
Chris