Hello Patrick,
I'm about to review merge the patches from the moblin/master branch into our
libsynthesis repo - thanks to your earlier cleanup of the history it's a really
straightforward task, thanks for that!
The only change that I'm not sure about is:
71aef21545 (TDebugLoggerBase::DebugOpenBlock(): unused va_list)
Doesn't the "static" prefix make that argument a global scope variable, and
thus make the routine non-reentrant and non-threadsafe?
Just because in another commit, you point out that a va_list can't be re-used,
which IMHO implies it is changed when it is processed by vsprintf. Of course,
in this case with the list being empty we can be reasonably safe it is not
changed or a change would not have any effect, even if multiple threads should
try it simultaneously. Still, is that clean enough?
Second - I don't really see why declaring it static would mean anything for
initialisation (apart from the fact that gcc seems not to be able to figure out
it should complain about it then) - C-ish stuff is not implicitly initialized,
neither in global scope nor on the stack.
I tried to initialize va by using va_start() on it, however gcc rejects this in
a function which has no ... in the declaration. We could add a ... just to make
gcc happy. Not nice either.
So my proposal would be just to call
DebugOpenBlock(blockname,title,collapsed,fmt,...) with fmt=NULL and no optional
arguments instead of DebugVOpenBlock:
// open new Block without attribute list
void TDebugLoggerBase::DebugOpenBlock(cAppCharP aBlockName, cAppCharP
aBlockTitle, bool aCollapsed)
{
// we need a format and debug not completely off
if (getMask() && aBlockName) {
DebugOpenBlock(aBlockName,aBlockTitle,aCollapsed,NULL);
}
} // TDebugLoggerBase::DebugOpenBlock
This is completely safe and we don't have to mess with the fact that apparently
there's no proper way to initialize an empty va_list.
Lukas Zeller ([email protected])
-
Synthesis AG, SyncML Solutions & Sustainable Software Concepts
[email protected], http://www.synthesis.ch
_______________________________________________
os-libsynthesis mailing list
[email protected]
http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis
