Question, and an Update for Command Expansion Patch

Andreas Ericsson ae at op5.se
Fri Nov 5 16:46:52 CET 2010


On 11/05/2010 03:58 PM, Jochen Bern wrote:
> On 08/31/2010 04:19 PM, Jochen Bern wrote:
>> The whitespace detection is still not quite finished because I hit a
>> snag in the existing code, namely, in cgi/cgiutils.c::url_encode() :
> 
> In the process of touching up 3.2.3 (CVS HEAD as of today) to include my
> local patches, I had a look at the references to url_encode(); I don't
> think that any reference *other* than the Command Expansion will ever
> feed control characters into it. Thus, I fixed the "is a space"
> comparison and removed the workaround in CE.
> 
> Also included in the attached patch:
> -- corrected typo in "entry" hyperlink (thanks to Alexey Dvoryanchikov,
>     via Icinga / Michael Friedrich)
> -- added hyperlinks to the CE of (active) check commands and event
>     handlers into the extinfo.cgi pages of hosts and services
> 

Please split patches into logical units so that one patch does one thing.
I won't touch patches that "also do this, and while we're at it fix this
and that and that". It makes a mess of history and makes bugfixing them
later a lot harder.

If you want them in, fix them up and write proper commit messages for
them, stating What Why and, if it's complex, How. That is, explain what
the patch does, why that is a good thing and anything else in particular
one needs to know about it when looking at the history.

I'd prefer if you write them against git at git://git.op5.org/nagios.git
(which I keep up to date with my tree) and then email patches with
git-send-email, Cc'ing me and To'ing this list.

One patch at a time in separate emails with a brief explanation would
work too though, but then I'll take them as and when I have time instead
of just applying them directly if they make sense.

> -------
> 
> While I'm writing: I intend to streamline some more patches so that they
> can be included into the authoritative source, but need to be activated
> by explicit request. I'm thinking along the lines of
> 	CFLAGS="-DFEATURE_XY"
> and optionally adding support into configure, pretty much like
> 	--with-feature-xy
> Or is there an established other syntax/format/... to do this?
> 

Do they really need to be configured at compile-time? Otherwise, add a
config option to cgi.cfg or nagios.cfg (whichever is appropriate) and
determine it at runtime instead.

-- 
Andreas Ericsson                   andreas.ericsson at op5.se
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.

------------------------------------------------------------------------------
The Next 800 Companies to Lead America's Growth: New Video Whitepaper
David G. Thomson, author of the best-selling book "Blueprint to a 
Billion" shares his insights and actions to help propel your 
business during the next growth cycle. Listen Now!
http://p.sf.net/sfu/SAP-dev2dev




More information about the Developers mailing list