From 48dca1ee9c558d87266178e82447fe6a0aa848c7 Mon Sep 17 00:00:00 2001 From: Sergey Linev Date: Wed, 14 May 2025 16:03:02 +0200 Subject: [PATCH] [roottest] fix leak by using TSystem::ConcatFileName Return value must be released. --- roottest/root/io/newstl/TestHelpers.C | 16 ++-- roottest/root/io/newstl/test.C | 37 +++++---- .../root/tree/cloning/runtreeCloneTest2.C | 75 ++++++++++--------- 3 files changed, 70 insertions(+), 58 deletions(-) diff --git a/roottest/root/io/newstl/TestHelpers.C b/roottest/root/io/newstl/TestHelpers.C index ea7223185da4a..398b7a6b7d37e 100644 --- a/roottest/root/io/newstl/TestHelpers.C +++ b/roottest/root/io/newstl/TestHelpers.C @@ -7,7 +7,7 @@ #include void fillListOfDir(const TString &directory, TList &l) { - + void *dir = gSystem->OpenDirectory(directory); const char *file = 0; @@ -25,11 +25,17 @@ void fillListOfDir(const TString &directory, TList &l) { TString s = file; if ( (basename!=file) && s.Index(re) == kNPOS) continue; - TString dirname = file; - if (directory != ".") - dirname = gSystem->ConcatFileName(directory, file); + TString dirname = file; + if (directory != ".") { + auto _dirname = gSystem->ConcatFileName(directory, file); + dirname = _dirname; + delete [] _dirname; + } + + auto _vfile = gSystem->ConcatFileName(dirname, "vector.root"); + TString vfile = _vfile; + delete [] _vfile; - TString vfile = gSystem->ConcatFileName(dirname, "vector.root"); if (gSystem->GetPathInfo(vfile,(Long_t*)0,(Long_t*)0,(Long_t*)0,0)==0) { l.Add(new TObjString(dirname)); } else { diff --git a/roottest/root/io/newstl/test.C b/roottest/root/io/newstl/test.C index c2184041a2ee4..da1897072510c 100644 --- a/roottest/root/io/newstl/test.C +++ b/roottest/root/io/newstl/test.C @@ -31,13 +31,15 @@ template void write(const char *testname, int nEntry = 3) { gSystem->Unlink("latest"); gSystem->Symlink(dirname,"latest"); - TString filename = gSystem->ConcatFileName(dirname, testname ); + auto _filename = gSystem->ConcatFileName(dirname, testname ); + TString filename = _filename; + delete [] _filename; filename += ".root"; TFile *file = new TFile(filename,"RECREATE","stl test file",0); HolderClass *holder = new HolderClass( 0 ); - + // Write(file,"scalar",holder->fScalar) // Write(file,"object",holder->fObject) // Write(file,"nested",holder->fNested) @@ -64,7 +66,7 @@ template void write(const char *testname, int nEntry = 3) { tree->Branch("scalar2.",&(holder->fScalarPtr),32000,2); tree->Branch("scalar99.",&(holder->fScalarPtr),32000,99); } - + TClass *clo = gROOT->GetClass(typeid(holder->fObject)); if (!clo) { TestError("TreeBuilding", Form("Writing holder class: Missing class for %s", @@ -80,7 +82,7 @@ template void write(const char *testname, int nEntry = 3) { if (!cln) { TestError("TreeBuilding", Form("Writing holder class: Missing class for %s", typeid(holder->fNested).name())); - } else { + } else { tree->Branch("nested0." ,&(holder->fNestedPtr),32000,0); tree->Branch("nested1." ,&(holder->fNestedPtr),32000,1); tree->Branch("nested2." ,&(holder->fNestedPtr),32000,2); @@ -105,14 +107,14 @@ template bool verifyBranch(const char *testname, TTree *chai TestError("treeReading",Form("Missing branch: %s",bname)); return false; } - + if (branch->InheritsFrom("TBranchObject")) { TLeafObject *tbo = dynamic_cast(branch->GetListOfLeaves()->At(0)); holder = (HolderClass*)(tbo->GetObject()); if (holder==0) { TestError("treeReading",Form("BranchObject %s with holder == 0!",bname)); - return false; + return false; } } else { add = (HolderClass**)branch->GetAddress(); @@ -128,7 +130,7 @@ template bool verifyBranch(const char *testname, TTree *chai case 3: p = (void**) &(gHolder->fNestedPtr); *p = ((TBranchElement*)branch)->GetObject(); break; } } - + int splitlevel = branch->GetSplitLevel(); switch (type) { @@ -136,7 +138,7 @@ template bool verifyBranch(const char *testname, TTree *chai case 1: return gHolder->VerifyScalarPtr(chain->GetTree()->GetReadEntry(),Form("%s %s",testname,bname),splitlevel); case 2: return gHolder->VerifyObjectPtr(chain->GetTree()->GetReadEntry(),Form("%s %s",testname,bname),splitlevel); case 3: return gHolder->VerifyNestedPtr(chain->GetTree()->GetReadEntry(),Form("%s %s",testname,bname),splitlevel); - default: + default: TestError("treeReading",Form("Unknown type %d in verifyBranch",type)); return false; } @@ -145,10 +147,13 @@ template bool verifyBranch(const char *testname, TTree *chai template bool read(const char *dirname, const char *testname, int nEntry, bool current) { HolderClass *holder = 0; bool result = true; - bool testingTopLevelVectors = true; + bool testingTopLevelVectors = true; + - TString filename = gSystem->ConcatFileName(dirname, testname ); + auto _filename = gSystem->ConcatFileName(dirname, testname ); + TString filename = _filename; filename += ".root"; + delete [] _filename; if (!current && gSystem->AccessPathName(filename, kFileExists)) { // when reading old directory a missing files is not an error. @@ -200,16 +205,16 @@ template bool read(const char *dirname, const char *testname result &= verifyBranch(testname,chain,"scalar1",1); result &= verifyBranch(testname,chain,"scalar2",1); result &= verifyBranch(testname,chain,"scalar99",1); - + result &= verifyBranch(testname,chain,"object0",2); result &= verifyBranch(testname,chain,"object1",2); result &= verifyBranch(testname,chain,"object2",2); result &= verifyBranch(testname,chain,"object99",2); - + result &= verifyBranch(testname,chain,"nested0",3); result &= verifyBranch(testname,chain,"nested1",3); result &= verifyBranch(testname,chain,"nested2",3); - result &= verifyBranch(testname,chain,"nested99",3); + result &= verifyBranch(testname,chain,"nested99",3); } } return result; @@ -217,7 +222,7 @@ template bool read(const char *dirname, const char *testname template bool read(const char *testname, int nEntry = 0, bool readother = false) { - // for each dirname + // for each dirname TString dirname = gROOT->GetVersion(); dirname.ReplaceAll(".","-"); dirname.ReplaceAll("/","-"); @@ -229,12 +234,12 @@ template bool read(const char *testname, int nEntry = 0, boo TList listOfDirs; listOfDirs.SetOwner(kTRUE); fillListOfDir(listOfDirs); - + TIter next(&listOfDirs); while (TObjString *dir = (TObjString*)next()) { if (dirname != dir->GetName()) { std::cout << "Testing older file format from: " << dir->GetName() << std::endl; - result &= read(dir->GetName(),testname, nEntry, /*current=*/ false); + result &= read(dir->GetName(),testname, nEntry, /*current=*/ false); } } } diff --git a/roottest/root/tree/cloning/runtreeCloneTest2.C b/roottest/root/tree/cloning/runtreeCloneTest2.C index 3297f4d8abd30..a7d7858fda5bc 100644 --- a/roottest/root/tree/cloning/runtreeCloneTest2.C +++ b/roottest/root/tree/cloning/runtreeCloneTest2.C @@ -2,7 +2,7 @@ NOTE: This macro is kept for back compatibility only. Use instead the executable $ROOTSYS/bin/hadd - + This macro will add histograms from a list of root files and write them to a target root file. The target file is newly created and must not be identical to one of the source files. @@ -13,13 +13,13 @@ Date: 13.2.2001 This code is based on the hadd.C example by Rene Brun and Dirk Geppert, which had a problem with directories more than one level deep. (see macro hadd_old.C for this previous implementation). - -The macro from Sven has been enhanced by + +The macro from Sven has been enhanced by Anne-Sylvie Nicollerat to automatically add Trees (via a chain of trees). - + To use this macro, modify the file names in function hadd. - + NB: This macro is provided as a tutorial. Use $ROOTSYS/bin/hadd to merge many histogram files @@ -46,67 +46,68 @@ void runtreeCloneTest2(const char* outFile = "pion_merged.root", //here's where const char* inputFileSubstring = ".root") //only files ending with this in the name will be opened for merging { FileList = new TList(); - + Target = TFile::Open(outFile, "RECREATE" ); - + TString fileString(inputFileSubstring); TString inputDir(theInputDir); - + //---- Loop over directory entries, select files ----// - + void *pDir = gSystem->OpenDirectory(inputDir.Data()); - + char* cFileName; while( (cFileName = (char*) gSystem->GetDirEntry(pDir)) ) { - + TString fileName(cFileName); if(fileName.EndsWith( fileString.Data() )) { - TString fullName = gSystem->ConcatFileName(inputDir,fileName); + auto fullName = gSystem->ConcatFileName(inputDir,fileName); FileList->Add( TFile::Open(fullName)); - cout << " Adding " << fullName.Data() << endl; + cout << " Adding " << fullName << endl; + delete [] fullName; } } cout <<"\n\nDone collecting files. begin merging\n"<GetPath() << endl; TString path( (char*)strstr( target->GetPath(), ":" ) ); path.Remove( 0, 2 ); - + TFile *first_source = (TFile*)sourcelist->First(); first_source->cd( path ); TDirectory *current_sourcedir = gDirectory; //gain time, do not add the objects in the list in memory Bool_t status = TH1::AddDirectoryStatus(); TH1::AddDirectory(kFALSE); - + // loop over all keys in this directory TChain *globChain = 0; TIter nextkey( current_sourcedir->GetListOfKeys() ); TKey *key, *oldkey=0; while ( (key = (TKey*)nextkey())) { - + //keep only the highest cycle number for each key if (oldkey && !strcmp(oldkey->GetName(),key->GetName())) continue; - + // read object from first source file first_source->cd( path ); TObject *obj = key->ReadObj(); - + if ( obj->IsA()->InheritsFrom( "TH1" ) ) { // descendant of TH1 -> merge it - + // cout << "Merging histogram " << obj->GetName() << endl; TH1 *h1 = (TH1*)obj; - + // loop over all source files and add the content of the // correspondant histogram to the one pointed to by "h1" TFile *nextsource = (TFile*)sourcelist->After( first_source ); while ( nextsource ) { - + // make sure we are at the correct directory level by cd'ing to path nextsource->cd( path ); cout <<"merging histogram from file:\t"<GetName()<Add( h2 ); delete h2; } - + nextsource = (TFile*)sourcelist->After( nextsource ); } } else if ( obj->IsA()->InheritsFrom( "TTree" ) ) { - + // loop over all source files create a chain of Trees "globChain" const char* obj_name= obj->GetName(); - + globChain = new TChain(obj_name); globChain->Add(first_source->GetName()); TFile *nextsource = (TFile*)sourcelist->After( first_source ); // const char* file_name = nextsource->GetName(); // cout << "file name " << file_name << endl; while ( nextsource ) { - + globChain->Add(nextsource->GetName()); nextsource = (TFile*)sourcelist->After( nextsource ); } - + } else if ( obj->IsA()->InheritsFrom( "TDirectory" ) ) { // it's a subdirectory - + cout << "Found subdirectory " << obj->GetName() << endl; - + // create a new subdir of same name and title in the target file target->cd(); TDirectory *newdir = target->mkdir( obj->GetName(), obj->GetTitle() ); - + // newdir is now the starting point of another round of merging // newdir still knows its depth within the target file via // GetPath(), so we can still figure out where we are in the recursion MergeRootfile( newdir, sourcelist ); - + } else { - + // object is of no type that we know or can handle - cout << "Unknown object type, name: " + cout << "Unknown object type, name: " << obj->GetName() << " title: " << obj->GetTitle() << endl; } - + // now write the merged histogram (which is "in" obj) to the target file // note that this will just store obj in the current directory level, // which is not persistent until the complete directory itself is stored // by "target->Write()" below if ( obj ) { target->cd(); - + //!!if the object is a tree, it is stored in globChain... if(obj->IsA()->InheritsFrom( "TTree" )) globChain->Merge(target->GetFile(),0,"keep fast"); else obj->Write( key->GetName() ); } - + } // while ( ( TKey *key = (TKey*)nextkey() ) ) - + // save modifications to target file target->SaveSelf(kTRUE); TH1::AddDirectory(status);