Index: /trunk/MagicSoft/Mars/Changelog =================================================================== --- /trunk/MagicSoft/Mars/Changelog (revision 5712) +++ /trunk/MagicSoft/Mars/Changelog (revision 5713) @@ -23,8 +23,4 @@ 2005/01/05 Thomas Bretz - * mbase/MStatusDisplay.cc: - - fixed some lines in pulldown-menus - - add TPads to ListOfCleanups in ClonePad (workaround!) - * mcalib/MCalibrationBlindPix.cc, mcalib/MCalibrationChargeCalc.cc, mhcalib/MHCalibrationRelTimeCam.cc: @@ -41,4 +37,46 @@ - made sure that LoGain is flagged non-valid if no lo-gain exists - push time back at the edge of the allowed limits if outside + + * mbase/MGList.[h,cc]: + - introduced new functions Delete and DeletePictures + - SetBit(kMustCleanup) in constructor. Otherwise it is not + correctly removed from ListOfCleanups + + * mbase/MLog.cc: + - SetBit(kMustCleanup) if adding a plugin to make sure it is cleaned + from the list + + * mbase/MParContainer.[h,cc]: + - replaced return type of GetDescriptor by TString. The old version + was terribly wrong! The returned memory wasn't removed as soon + as the function returned. + + * mbase/MStatusDisplay.[h,cc]: + - for security (thread safty) replaced all Form() by MString::Form() + - implemented a more secure scheme of Closing (Clone()) this was + a hint given by valgrind + - set kMustCleanup for all new object in ClonePad not only kCanDelete + - fixed some lines in pulldown-menus + - add TPads to ListOfCleanups in ClonePad (workaround!) + + * mbase/MString.[h,cc]: + - implemented a new static Form() function returning a copy of + the result + + * mbase/MTask.[h,cc]: + - replaced return type of GetDescriptor by TString. The old version + was terribly wrong! The returned memory wasn't removed as soon + as the function returned. + - Made GetDescriptor thread safe using MString::Form + + * mfileio/MWriteRootFile.cc: + - Use GetDescriptor().Data() when using any kind of printf function + + * mhbase/MFillH.cc: + - replaced string concatenation in all constructors by a Form + (also suggested by valgrind) + + * mhcalib/MHCalibrationRelTimeCam.cc: + - made some output fit into 80col boundary Index: /trunk/MagicSoft/Mars/mbase/MGList.cc =================================================================== --- /trunk/MagicSoft/Mars/mbase/MGList.cc (revision 5712) +++ /trunk/MagicSoft/Mars/mbase/MGList.cc (revision 5713) @@ -54,5 +54,8 @@ MGList::MGList() : TList() { + // Make sure that all object deleted are also deleted from this list gROOT->GetListOfCleanups()->Add(this); + // Make sure that this doesn't remain in ListOfCleanups after deletion + SetBit(kMustCleanup); } @@ -63,4 +66,13 @@ // MGList::~MGList() +{ + DeletePictures(); +} + +// -------------------------------------------------------------------------- +// +// Free/Delete all TGPicture store in list. +// +void MGList::DeletePictures() { TObject *obj; @@ -82,4 +94,14 @@ // -------------------------------------------------------------------------- // +// Delete all pictures. Call TList::Delete. +// +void MGList::Delete(Option_t *option) +{ + DeletePictures(); + TList::Delete(option); +} + +// -------------------------------------------------------------------------- +// // Does a dynamic cast from a TObject to a TGWidget. This is necesary // if a class derived from TObject inherits also from TGWidget and Index: /trunk/MagicSoft/Mars/mbase/MGList.h =================================================================== --- /trunk/MagicSoft/Mars/mbase/MGList.h (revision 5712) +++ /trunk/MagicSoft/Mars/mbase/MGList.h (revision 5713) @@ -16,4 +16,5 @@ void AddPicture(TGPicture *pic, const char *name); + void DeletePictures(); public: @@ -23,4 +24,6 @@ void Add(TObject *obj); void Add(TObject *obj, Option_t *opt); + + void Delete(Option_t *option=""); const TGPicture *GetPicture(const char *name); Index: /trunk/MagicSoft/Mars/mbase/MLog.cc =================================================================== --- /trunk/MagicSoft/Mars/mbase/MLog.cc (revision 5712) +++ /trunk/MagicSoft/Mars/mbase/MLog.cc (revision 5713) @@ -117,5 +117,5 @@ #undef DEBUG -//#define DEBUG +#define DEBUG @@ -701,8 +701,11 @@ // is automatically removed from the list of active plugins. // -// If MLog should take the ownership cal plug->SetBit(kCanDelete); +// If MLog should take the ownership call plug->SetBit(kCanDelete); // void MLog::AddPlugin(MLogPlugin *plug) { fPlugins->Add(plug); -} + + // Make sure that it is recursively deleted from all objects in ListOfCleanups + plug->SetBit(kMustCleanup); +} Index: /trunk/MagicSoft/Mars/mbase/MParContainer.cc =================================================================== --- /trunk/MagicSoft/Mars/mbase/MParContainer.cc (revision 5712) +++ /trunk/MagicSoft/Mars/mbase/MParContainer.cc (revision 5713) @@ -208,5 +208,5 @@ // class name it returns the object name and in []-brackets the class name. // -const char *MParContainer::GetDescriptor() const +const TString MParContainer::GetDescriptor() const { return GetDescriptor(*this); @@ -218,5 +218,5 @@ // class name it returns the object name and in []-brackets the class name. // -const char *MParContainer::GetDescriptor(const TObject &o) +const TString MParContainer::GetDescriptor(const TObject &o) { // @@ -227,5 +227,5 @@ MString desc; desc.Print("%s [%s]", o.GetName(), o.ClassName()); - return (TString)o.GetName()==o.ClassName() ? o.ClassName() : desc.Data(); + return (TString)o.GetName()==o.ClassName() ? o.ClassName() : desc; } Index: /trunk/MagicSoft/Mars/mbase/MParContainer.h =================================================================== --- /trunk/MagicSoft/Mars/mbase/MParContainer.h (revision 5712) +++ /trunk/MagicSoft/Mars/mbase/MParContainer.h (revision 5713) @@ -74,7 +74,7 @@ virtual void FillBuffer(char *&buffer); - static const char *GetDescriptor(const TObject &o); + static const TString GetDescriptor(const TObject &o); - virtual const char *GetDescriptor() const; + virtual const TString GetDescriptor() const; virtual const TString GetUniqueName() const; virtual const char *GetName() const { return fName.Data(); } Index: /trunk/MagicSoft/Mars/mbase/MStatusDisplay.cc =================================================================== --- /trunk/MagicSoft/Mars/mbase/MStatusDisplay.cc (revision 5712) +++ /trunk/MagicSoft/Mars/mbase/MStatusDisplay.cc (revision 5713) @@ -91,4 +91,6 @@ #include // TGHProgressBar #include // TRootEmbeddedCanvas + +#include "MString.h" #include "MLog.h" // MLog @@ -336,5 +338,5 @@ // Add MARS version - TGLabel *l = new TGLabel(f, Form("Official Release: V%s", MARSVER)); + TGLabel *l = new TGLabel(f, MString::Form("Official Release: V%s", MARSVER)); fList->Add(l); @@ -347,5 +349,5 @@ // Add root version - l = new TGLabel(f, Form("Using ROOT v%s", ROOTVER)); + l = new TGLabel(f, MString::Form("Using ROOT v%s", ROOTVER)); fList->Add(l); @@ -575,5 +577,5 @@ void MStatusDisplay::SetStatusLine2(const MParContainer &cont) { - SetStatusLine2(Form("%s: %s", cont.GetDescriptor(), cont.GetTitle())); + SetStatusLine2(MString::Form("%s: %s", cont.GetDescriptor().Data(), cont.GetTitle())); } @@ -601,5 +603,5 @@ // from the list in the destructor. // - gROOT->GetListOfSpecials()->Add(this); +// gROOT->GetListOfSpecials()->Add(this); fFont = gVirtualX->LoadQueryFont("7x13bold"); @@ -694,5 +696,5 @@ // - // Delete the list list of canvases used in batch mode + // Delete the list of canvases used in batch mode // instead of the Root Embedded Canvases in the TGTab // @@ -971,5 +973,5 @@ { // Tell the X-Requester how to call this method - TString str = Form("%d", (ULong_t)f); + TString str = MString::Form("%d", (ULong_t)f); TMethodCall call(IsA(), "UpdateTab", "NULL"); @@ -1063,5 +1065,5 @@ gSystem->Unlink(name); - SetStatusLine2(Form("Done (%dpage(s))", pages)); + SetStatusLine2(MString::Form("Done (%dpage(s))", pages)); return pages; @@ -1177,5 +1179,6 @@ case kFileExit: if (id==kFileExit || id==kFileClose) - CloseWindow(); + if (Close()) + delete this; fStatus = (Status_t)id; return kTRUE; @@ -1489,5 +1492,5 @@ } -void MStatusDisplay::CloseWindow() +Bool_t MStatusDisplay::Close() { // Got close message for this MainFrame. Calls parent CloseWindow() @@ -1512,14 +1515,13 @@ if (fIsLocked<=0 && IsOnHeap()) - { - //gLog << dbg << "delete " << fName << ";" << endl; - delete this; - // DO NOT EXECUTE ANY COMMAND AFTER this! - } - else - { - fStatus = kFileExit; - //gLog << dbg << fName << ".fStatus=kFileExit;" << endl; - } + return kTRUE; + + fStatus = kFileExit; + return kFALSE; +} + +void MStatusDisplay::CloseWindow() +{ + Close(); } @@ -1621,8 +1623,8 @@ // Clone also important bits (FIXME: Is this correct) - clone->SetBit(obj->TestBits(kMustCleanup|kCannotPick|kNoContextMenu)); + clone->SetBit(obj->TestBits(kCannotPick|kNoContextMenu)); // Now make sure that the clones are deleted at a later time - clone->SetBit(kCanDelete); + clone->SetBit(kCanDelete|kMustCleanup); // FIXME: This is a workaround for the problem with the MAstroCatalog in @@ -2015,5 +2017,5 @@ } - SetStatusLine2(Form("Tab #%d", i)); + SetStatusLine2(MString::Form("Tab #%d", i)); // @@ -2078,5 +2080,5 @@ ps.TextNDC(0.5, 1.015, TString("MARS - Magic Analysis and Reconstruction Software - ")+d.AsString()); ps.SetTextAlign(31); // right top - ps.TextNDC(1, 1.015, Form("Page No.%i (%i) ", page++, i)); + ps.TextNDC(1, 1.015, MString::Form("Page No.%i (%i) ", page++, i)); line.PaintLineNDC(0, 1.01, 1, 1.01); @@ -2118,5 +2120,5 @@ *fLog << inf << "done." << endl; - SetStatusLine2(Form("Done (%dpages)", page-1)); + SetStatusLine2(MString::Form("Done (%dpages)", page-1)); return page-1; @@ -2165,5 +2167,5 @@ } - SetStatusLine2(Form("Tab #%d", i)); + SetStatusLine2(MString::Form("Tab #%d", i)); // @@ -2249,5 +2251,5 @@ } - SetStatusLine2(Form("Tab #%d", i)); + SetStatusLine2(MString::Form("Tab #%d", i)); // @@ -2460,5 +2462,16 @@ Bool_t MStatusDisplay::HandleEvent(Event_t *event) { - Bool_t rc = TGMainFrame::HandleEvent(event); + // Instead of doing this in CloseWindow (called from HandleEvent) + // we do it here. This makes sure, that handle event doesn't + // execute code after deleting this. + if (event->fType==kDestroyNotify) + { + if (Close()) + delete this; +// Close(); + return kTRUE; + } + + const Bool_t rc = TGMainFrame::HandleEvent(event); // Index: /trunk/MagicSoft/Mars/mbase/MStatusDisplay.h =================================================================== --- /trunk/MagicSoft/Mars/mbase/MStatusDisplay.h (revision 5712) +++ /trunk/MagicSoft/Mars/mbase/MStatusDisplay.h (revision 5713) @@ -110,4 +110,5 @@ Bool_t ProcessMessageTextview(Long_t submsg, Long_t mp1, Long_t mp2); Bool_t ProcessMessageUser(Long_t submsg, Long_t mp1, Long_t mp2); + Bool_t Close(); void CloseWindow(); Bool_t HandleConfigureNotify(Event_t *); Index: /trunk/MagicSoft/Mars/mbase/MString.cc =================================================================== --- /trunk/MagicSoft/Mars/mbase/MString.cc (revision 5712) +++ /trunk/MagicSoft/Mars/mbase/MString.cc (revision 5713) @@ -59,9 +59,6 @@ // string.Print(" MyString has %d bytes ", 128).Strip(TString::kBoth); // -MString &MString::Print(const char *fmt, ...) +MString &MString::Print(const char *fmt, va_list &ap) { - va_list ap; - va_start(ap, fmt); - Int_t n=256; @@ -87,2 +84,47 @@ return *this; } + +// -------------------------------------------------------------------------- +// +// Thread safe replacement for Form, use it like: +// +// MString string; +// string.Print("MyString has %d bytes", 128); +// +// As a special feature the function returns the reference to the MString +// so that you can directly work with it, eg. +// +// string.Print(" MyString has %d bytes ", 128).Strip(TString::kBoth); +// +MString &MString::Print(const char *fmt, ...) +{ + va_list ap; + va_start(ap, fmt); + + return Print(fmt, ap); +} + +// -------------------------------------------------------------------------- +// +// Thread safe replacement for Form, use it like: +// +// MString string; +// string.Print("MyString has %d bytes", 128); +// +// As a special feature the function returns the reference to the MString +// so that you can directly work with it, eg. +// +// string.Print(" MyString has %d bytes ", 128).Strip(TString::kBoth); +// +// The static version of this function returns a copy(!) of the resulting +// M/TString. +// +MString MString::Form(const char *fmt, ...) +{ + va_list ap; + va_start(ap, fmt); + + MString ret; + ret.Print(fmt, ap); + return ret; +} Index: /trunk/MagicSoft/Mars/mbase/MString.h =================================================================== --- /trunk/MagicSoft/Mars/mbase/MString.h (revision 5712) +++ /trunk/MagicSoft/Mars/mbase/MString.h (revision 5713) @@ -12,5 +12,7 @@ { public: + MString &Print(const char *fmt, va_list &ap); MString &Print(const char *fmt, ...); + static MString Form(const char *fmt, ...); ClassDef(MString, 1) // Tool to make Form() thread safe against other TStrings Index: /trunk/MagicSoft/Mars/mbase/MTask.cc =================================================================== --- /trunk/MagicSoft/Mars/mbase/MTask.cc (revision 5712) +++ /trunk/MagicSoft/Mars/mbase/MTask.cc (revision 5713) @@ -101,4 +101,6 @@ #include // TStopwatch +#include "MString.h" + #include "MLog.h" #include "MLogManip.h" @@ -335,5 +337,5 @@ // name (eg. ;1) // -const char *MTask::GetDescriptor() const +const TString MTask::GetDescriptor() const { // @@ -343,9 +345,9 @@ // if (fName==ClassName()) - return fSerialNumber==0 ? ClassName() : Form("%s;%d", ClassName(), fSerialNumber); + return fSerialNumber==0 ? ClassName() : MString::Form("%s;%d", ClassName(), fSerialNumber); return fSerialNumber>0 ? - Form("%s;%d [%s]", fName.Data(), fSerialNumber, ClassName()) : - Form("%s [%s]", fName.Data(), ClassName()); + MString::Form("%s;%d [%s]", fName.Data(), fSerialNumber, ClassName()) : + MString::Form("%s [%s]", fName.Data(), ClassName()); } Index: /trunk/MagicSoft/Mars/mbase/MTask.h =================================================================== --- /trunk/MagicSoft/Mars/mbase/MTask.h (revision 5712) +++ /trunk/MagicSoft/Mars/mbase/MTask.h (revision 5713) @@ -89,5 +89,5 @@ Byte_t GetSerialNumber() const { return fSerialNumber; } - const char *GetDescriptor() const; + const TString GetDescriptor() const; // Task execution statistics Index: /trunk/MagicSoft/Mars/mfileio/MWriteRootFile.cc =================================================================== --- /trunk/MagicSoft/Mars/mfileio/MWriteRootFile.cc (revision 5712) +++ /trunk/MagicSoft/Mars/mfileio/MWriteRootFile.cc (revision 5713) @@ -405,5 +405,5 @@ const char *cname = cont->GetName(); const char *tname = entry->GetName(); - const TString ttitle(Form("Tree containing %s", cont->GetDescriptor())); + const TString ttitle(Form("Tree containing %s", cont->GetDescriptor().Data())); // Index: /trunk/MagicSoft/Mars/mhbase/MFillH.cc =================================================================== --- /trunk/MagicSoft/Mars/mhbase/MFillH.cc (revision 5712) +++ /trunk/MagicSoft/Mars/mhbase/MFillH.cc (revision 5713) @@ -166,9 +166,9 @@ return; - fTitle = "Fill " + fHName; + fTitle = Form("Fill %s", fHName.Data()); if (fParContainerName.IsNull()) return; - fTitle += " from " + fParContainerName; + fTitle += Form(" from %s", fParContainerName.Data()); } @@ -209,5 +209,5 @@ if (!title) - fTitle = "Fill " + fHName + " from " + par->GetDescriptor(); + fTitle = Form("Fill %s from %s", fName.Data(), par->GetDescriptor().Data()); } @@ -236,9 +236,9 @@ return; - fTitle = (TString)"Fill " + hist->GetDescriptor(); + fTitle = Form("Fill %s", hist->GetDescriptor().Data()); if (!par) return; - fTitle += " from " + fParContainerName; + fTitle += Form(" from %s", fParContainerName.Data()); } @@ -265,5 +265,5 @@ if (!title) - fTitle = (TString)"Fill " + hist->GetDescriptor() + " from " + par->GetDescriptor(); + fTitle = Form("Fill %s from %s", hist->GetDescriptor().Data(), par->GetDescriptor().Data()); } Index: /trunk/MagicSoft/Mars/mhcalib/MHCalibrationRelTimeCam.cc =================================================================== --- /trunk/MagicSoft/Mars/mhcalib/MHCalibrationRelTimeCam.cc (revision 5712) +++ /trunk/MagicSoft/Mars/mhcalib/MHCalibrationRelTimeCam.cc (revision 5713) @@ -679,5 +679,5 @@ if (overflow > 0.0005*hist->GetEntries()) { - *fLog << warn << "HiGain Hist-overflow occurred " << overflow + *fLog << warn << "HiGain Hist-overflow " << overflow << " times in " << pix.GetName() << " (w/o saturation!) " << endl; } @@ -686,5 +686,5 @@ if (overflow > 0.0005*hist->GetEntries()) { - *fLog << warn << "HiGain Hist-underflow occurred " << overflow + *fLog << warn << "HiGain Hist-underflow " << overflow << " times in " << pix.GetName() << " (w/o saturation!) " << endl; }