Re: [Nagios-devel] Do not launch a shell for each check

[Andreas E. <ae...@op...>]

On 05/31/2010 04:28 PM, Matthieu Kermagoret wrote:
> Hi list,
> 

Hi Matt :)

> I'd like to propose a performance patch for Nagios that reduces the
> number of Nagios' descendant processes.
> 

Thanks. The idea behind the patch is good. The patch itself is not so
stellar though.

First off, the way you're building the command is far from optimal.
So far from it that it's quite easy to construct a command-line
that yields worse performance with your patch than forking a new
shell to take care of the command to run. A better approach is to
malloc() once and make sure the resulting buffer is large enough to
hold the resulting command-line and iterate over it once using a
simple "while (*p)" loop, discarding backslashes and handling quoting
as you go.

Secondly, it would be a lot better if the parse_command_line() function
automagically detects special shell characters (pipes, stdio/stdin/stderr
redirection, job-control stuff etc etc) and automagically used the shell
to run such commands. That would, barring bugs, make it possible to use
your patch with all existing Nagios configs without modifying those
configurations. Considering the fact that the standard notification
command includes a pipe, that's a pretty important thing to remember.

Thirdly, you define the macro WHITESPACES in two different locations,
which is error-prone.

Fourthly, the parse_command_line() function is, with this patch, only
used from base/checks.c, so it would be much better off residing in
that file, and scoped within that file only. I realize it could quite
easily be used at more places later, but then a separate patch moving
it out to global scope would be a lot better, since those patches may
not be accepted.

-- 
Andreas Ericsson                   and...@op...
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.