[Xorp-hackers] PATCH: Logging improvements, fix artificial deal for xorpsh commit.

[Bruce Simpson]

Ben Greear wrote:
> The attached patch has these improvements:
>
> 1)  Fix logging & tracing to show micro-seconds, greatly aids 
> debugging performance issues.
> 2)  Change some pass-by-value string arguments to const string& in 
> router-mgr.  This will improve
>     performance and a small bit of memory usage.
> 3)  Remove 1 second timeout in 'commit' path.  At best, the timeout 
> might have worked around
>     a race condition, but I can see no reason to leave it in.  I 
> tested with it set to zero
>     timeout and things work fine.  This makes commits last around 
> 200ms instead of 1.2ms, which is
>     a big improvement when scripting xorp.

Comments:
   * It should be possible to turn off the millisecond logging if 
desired. Whilst it's certainly a useful feature to have when debugging 
time contingent code, it does add clutter to the output.
    * Perhaps putting it under the other debug knobs in SConstruct would 
be a good idea?
   * %llu is not a portable format specifier, and 'unsigned long long' 
is not a portable type, please don't use them in portable code.
  * Perhaps the code which prints the timeval is a candidate for a 
function like xlog_localtime2string_short() ?

  * xlog_localtime2string_short() is still defined in xlog.c; so why comment out its prototype, are you getting warnings from the compiler?
  * A XorpTimer of 0 is a possible candidate for a XorpTask. I can't really delve further into that change at the moment, though.
  * Yes, it may be useful to constify the string arguments in those callback functions, but this change considered low priority.
  * Please avoid introducing unnecessary whitespace changes in diffs.
  

Can you please raise a Trac item for these suggested improvements?
I probably won't have time to look at the Router Manager in detail for at least 4 weeks.

Sorry for the bureaucracy... I appreciate you're doing what you can in the here and now to improve the code, however, it makes reviewing patches and applying them that much easier, and we do need to keep the code alignment and type clean, etc.

thanks,
BMS