[Openswan dev] [PATCH] remove satot calls when debug disabled

[David McCullough]

Jivin D. Hugh Redelmeier lays it down ...
> | From: David McCullough <davidm at snapgear.com>
> 
> | Here's a sample patch for some feedback.  Basically,  there are a lot of
> | satot calls that get invoked even when debug is off.  This has a
> | significant impact on VPN thoughput.
> 
> That is bad and should be fixed.
> 
> | If people feel the approach and reasoning is sound I'll produce a full patch
> | for all the satot calls that need it.  This is a cleaned up version of
> | what is already in the OCF patches.
> 
> In my opinion, your fix is not the most clear.  In some ways it is the
> minimal perturbation of the source, which is good if the patch were
> not to be adopted by the mainstream code.

My current aims were minimal code change,  while getting the
performance.

> In Pluto there was a macro in which to wrap up debug-conditional
> code.  Code like these calls to satot would be moved within the
> same conditional as the print.
> 
> Here's a random example from a 4 year old copy:
> 
>     DBG(DBG_PARSING,
> 	{
> 	    char buf[IDTOA_BUF];
> 
> 	    idtoa(&peer, buf, sizeof(buf));
> 	    DBG_log("Peer's ID is %s: '%s'",
> 		enum_show(&ident_names, id->isaid_idtype),
> 		buf);
> 	});
> 
> Why do I think that this kind of approach is better?
> 
> (1) the logic becomes more manifest
> 
> (2) the object code isn't bloated with redundant tests
> 
> (3) the source code does not have redundant tests
>     (if redundant tests get out of sync, problems ensue)

Another option I was considering was an satot wrapper that returned
either the SA string or "(error)",  since that is the most common use
of it for debug.  This would remove a lot of the conditional code in the
debug statements etc. and stop the satot calls being seperate to the
actual debug statement.  Of course this changed the code more that the
other solution ;-)

Something like this currently:

	sa_len = satot(...)
	...
	KLIPS_DEBUG(pfkey_debug, "sa is %s", sa_len ? sa : "(error)")

would become

	KLIPS_DEBUG(pfkey_debug, "sa is %s", satot_or_error(...))

Cheers,
Davidm

-- 
David McCullough, davidm at cyberguard.com.au, Custom Embedded Solutions + Security
Ph:+61 734352815 Fx:+61 738913630 http://www.uCdot.org http://www.cyberguard.com