[OPEN-ILS-DEV] PATCH: osrfConfig.C (dropping stderr messages)

[Dan Scott]

On 28/06/07, Bill Erickson <billserickson at gmail.com> wrote:
>
>
>
> On 6/28/07, Mike Rylander <mrylander at gmail.com> wrote:
> > On 6/27/07, Scott McKellar <mck9 at swbell.net> wrote:
> > > This patch undoes a previous patch.
> > >
> >
> > Applied.
> >
> > Related to this, I scanned the OpenSRF source for other direct uses of
> > stderr ... I found a LOT.  It's been far too long a day for me to
> > attack that now, but it's something we really should clean up, because
> > ...
> >
> > I've been thinking more about some quirks of OpenSRF, and it struck me
> > that a big one is just the fact that we even use std* after fork()ing
> > and setsid()ing.  That's generally a no-no, and can create all manner
> > of not-good things, such as trapping ssh sessions.
> >
> > So, while it's obviously important that we get messages to the user
> > ASAP, and that we make debugging new apps as easy as possible, I think
> > we need to discourage the practice in OpenSRF.
> >
> > To that end, I propose that we forcibly close stdin, stdout and stderr
> > inside libopensrf/utils.c:daemonize() after calling setsid().  This
> > will require some more checks in the logging code to make sure that
> > stderr is still around for logging, and if not ... well, like Scott
> > said, the admin is a poo-poo head for not providing either a log file
> > or the required syslog info, and the message will be lost.  Logging
> > directly to stderr from within an opensrf application will also
> > obviously do nothing in this case.
> >
> > As a side note, applications implemented the normal way, as shared
> > objects to be dlopen()d by the opensrf server, should not have (and
> > really, have never had, thus far) any expectation of access to the
> > std* fds.  They must use the logging infrastructure for non-response
> > output.
> >
> > I believe could be convince to make the forcible closing of said fds
> > dependent on some environmental factor -- an actual environment
> > variable or maybe a config file setting -- but I don't see much direct
> > value in this, personally.  A log file is generally more convenient
> > and would require exactly the same amount of effort (a config file
> > setting) to put in place.  The only place where this will come in very
> > handy is in troubleshooting mis-configured environments, especially
> > those without direct disk access for the opensrf user -- and for those
> > on this list, that could be a big benefit ...
> >
> > Anyway, I wanted to get the idea out here.  Does anyone see something
> > I'm missing here?  Any other ideas/directions to consider?
>
>
> +1 for removing raw print statements and closing stdin, stdout, and stderr
> in daemonize().  Any raw fprintf(stderr, ...) logging statements in the
> OpenSRF core code are just debugging artifacts.  There are also probably a
> lot in various test programs as well (testing JSON, testing Jabber, etc.),
> which we can ignore for now.

Well, raw print debugging artifacts can be useful; they were useful
once and potentially could be useful again. You could wrap them in
#ifdef OSRFDEBUG / #endif sections in case you or someone else
struggling to learn the code ever wanted to revive them. But as long
as they are disabled or gone from the normal mode of operation, I
don't feel strongly about how they get removed.

So +1 here, with some attitude.


-- 
Dan Scott
Laurentian University