From: Thomas Guyot-S. <de...@ae...> - 2010-06-18 12:20:52
|
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 10-06-01 05:21 AM, Andreas Ericsson wrote: > 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. Hey, I had that idea for quite some time but never got around implementing it. Thanks guys for getting to it ;) So if I may add something, my idea was to use a different command syntax using ! characters to separate arguments. for ex: command_exec \ $USER1/check_ping!-H!$HOSTADDRESS$!-w!$ARG1$!-c!$ARG2$!-p!1!-t!1 the ability to escape ! and \ would be useful too, and I think it would make it much simpler to code and use than any other "smart" method. The same could be done to nrpe, and this would allow a safe way to pass command arguments! Just my .2 cents... - -- Thomas -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkwbYr0ACgkQ6dZ+Kt5BchYxbgCg8RWkgpFu0KzuKIHLujaFUv63 q1cAoNfeRO5H96vIhH/BDhHy3MxIwbIQ =kErP -----END PGP SIGNATURE----- |