From b2138d2fc370f2b9238cdcbca01506aedeb2418e Mon Sep 17 00:00:00 2001
From: Gerardo Ganis <Gerardo.Ganis@cern.ch>
Date: Wed, 23 Jan 2013 10:01:49 +0000
Subject: [PATCH] From Dario, fixes for Coverity reports: In
TDataSetManagerAliEn/TAliEnFind: fixed Coverity issues 48630 to 58637: -
fixed a couple of minor memory leaks; - fixed use of invalidated iterator;
- added copy ctor and op=
In TProof.cxx: fixed a misspelled method name in debug messages
git-svn-id: http://root.cern.ch/svn/root/trunk@48378 27541ba8-7e3a-0410-8455-c3a389f83636
---
proof/proof/inc/TDataSetManagerAliEn.h | 35 +++-
proof/proof/src/TDataSetManagerAliEn.cxx | 239 ++++++++++++++++++-----
proof/proof/src/TProof.cxx | 8 +-
3 files changed, 218 insertions(+), 64 deletions(-)
diff --git a/proof/proof/inc/TDataSetManagerAliEn.h b/proof/proof/inc/TDataSetManagerAliEn.h
index 4843009c23a..511148a2a89 100644
--- a/proof/proof/inc/TDataSetManagerAliEn.h
+++ b/proof/proof/inc/TDataSetManagerAliEn.h
@@ -45,6 +45,7 @@ typedef enum { kDataRemote, kDataCache, kDataLocal } EDataMode;
class TAliEnFind : public TObject {
private:
+
TString fBasePath;
TString fFileName;
TString fTreeName;
@@ -55,23 +56,41 @@ class TAliEnFind : public TObject {
TString fSearchId;
TGridResult *fGridResult;
+ inline virtual void InvalidateSearchId();
+ inline virtual void InvalidateGridResult();
+
public:
- TAliEnFind(const TString &basePath, const TString &fileName,
+ TAliEnFind(const TString &basePath = "", const TString &fileName = "",
const TString &anchor = "", const Bool_t archSubst = kFALSE,
const TString &treeName = "", const TString ®exp = "");
+ TAliEnFind(const TAliEnFind &src);
+ TAliEnFind &operator=(const TAliEnFind &rhs);
+
virtual ~TAliEnFind();
- virtual TGridResult *GetGridResult();
- virtual const TString &GetBasePath() const { return fBasePath; };
- virtual const TString &GetFileName() const { return fFileName; };
- virtual const TString &GetTreeName() const { return fTreeName; };
- virtual const TPMERegexp *GetRegexp() const { return fRegexp; };
+ virtual TGridResult *GetGridResult(Bool_t forceNewQuery = kFALSE);
+
+ virtual const TString &GetBasePath() const { return fBasePath; };
+ virtual const TString &GetFileName() const { return fFileName; };
+ virtual const TString &GetAnchor() const { return fAnchor; };
+ virtual const TString &GetTreeName() const { return fTreeName; };
+ virtual Bool_t GetArchSubst() const { return fArchSubst; };
+ virtual const TPMERegexp *GetRegexp() const { return fRegexp; };
+
+ virtual void SetBasePath(const char *basePath);
+ virtual void SetFileName(const char *fileName);
+ virtual void SetAnchor(const char *anchor);
+ virtual void SetTreeName(const char *fileName);
+ virtual void SetArchSubst(Bool_t archSubst);
+ virtual void SetRegexp(const char *regexp);
+
virtual const char *GetSearchId();
- virtual TFileCollection *GetCollection();
+ virtual TFileCollection *GetCollection(Bool_t forceNewQuery = kFALSE);
+ virtual void Print(Option_t* opt = "") const;
- ClassDef(TAliEnFind, 0); // Interface to AliEn find command
+ ClassDef(TAliEnFind, 0); // Interface to the AliEn find command
};
diff --git a/proof/proof/src/TDataSetManagerAliEn.cxx b/proof/proof/src/TDataSetManagerAliEn.cxx
index 437dc3979dd..75222b6d036 100644
--- a/proof/proof/src/TDataSetManagerAliEn.cxx
+++ b/proof/proof/src/TDataSetManagerAliEn.cxx
@@ -37,6 +37,45 @@ TAliEnFind::TAliEnFind(const TString &basePath, const TString &fileName,
fRegexp = new TPMERegexp(regexp);
}
+//______________________________________________________________________________
+TAliEnFind::TAliEnFind(const TAliEnFind &src)
+{
+ // Copy constructor. Cached query result is not copied
+
+ fBasePath = src.fBasePath;
+ fFileName = src.fFileName;
+ fAnchor = src.fAnchor;
+ fArchSubst = src.fArchSubst;
+ fTreeName = src.fTreeName;
+ fRegexpRaw = src.fRegexpRaw;
+
+ if (src.fRegexp)
+ fRegexp = new TPMERegexp( *(src.fRegexp) );
+ else
+ fRegexp = NULL;
+
+ fGridResult = NULL;
+}
+
+//______________________________________________________________________________
+TAliEnFind &TAliEnFind::operator=(const TAliEnFind &rhs)
+{
+ // Assignment operator. Cached query result is not copied
+
+ fBasePath = rhs.fBasePath;
+ fFileName = rhs.fFileName;
+ fAnchor = rhs.fAnchor;
+ fArchSubst = rhs.fArchSubst;
+ fTreeName = rhs.fTreeName;
+
+ SetRegexp(rhs.fRegexpRaw);
+
+ InvalidateSearchId();
+ InvalidateGridResult();
+
+ return *this;
+}
+
//______________________________________________________________________________
TAliEnFind::~TAliEnFind()
{
@@ -47,12 +86,20 @@ TAliEnFind::~TAliEnFind()
}
//______________________________________________________________________________
-TGridResult *TAliEnFind::GetGridResult()
+TGridResult *TAliEnFind::GetGridResult(Bool_t forceNewQuery)
{
- // Query the AliEn grid
-
- if (fGridResult)
+ // Query the AliEn file catalog
+
+ if (fGridResult && !forceNewQuery) {
+ if (gDebug >= 1)
+ Info("GetGridResult", "Returning cached AliEn find results");
return fGridResult;
+ }
+ else if (gDebug >= 1) {
+ Info("GetGridResult", "Querying AliEn file catalog");
+ }
+
+ InvalidateGridResult();
if (!gGrid) {
TGrid::Connect("alien:");
@@ -65,6 +112,7 @@ TGridResult *TAliEnFind::GetGridResult()
}
fGridResult = gGrid->Query(fBasePath.Data(), fFileName.Data());
+ if (!fGridResult) return NULL;
if (fRegexp || fArchSubst || (fAnchor != "")) {
@@ -81,32 +129,33 @@ TGridResult *TAliEnFind::GetGridResult()
TObjString *os;
TString tUrl;
- while (( map = dynamic_cast<TMap *>(it.Next()) ) != NULL) {
-
- os = dynamic_cast<TObjString *>( map->GetValue("turl") );
- if (!os) continue;
- tUrl = os->String();
-
- if (fRegexp && (fRegexp->Match(tUrl) == 0)) {
- // Remove object if it does not match expression
- TObject *exmap = fGridResult->Remove(map);
- if (exmap) delete exmap; // Remove() does not delete
- }
-
- if (reArchSubst) {
- reArchSubst->Substitute(tUrl, substWith, kFALSE);
- os->SetString(tUrl.Data());
- }
- else if (fAnchor) {
- tUrl.Append("#");
- tUrl.Append(fAnchor);
- os->SetString(tUrl.Data());
+ while (( map = dynamic_cast<TMap *>(it.Next()) ) != NULL) {
+
+ os = dynamic_cast<TObjString *>( map->GetValue("turl") );
+ if (!os) continue;
+ tUrl = os->String();
+
+ if (fRegexp && (fRegexp->Match(tUrl) == 0)) {
+ // Remove object if it does not match expression
+ TObject *exmap = fGridResult->Remove(map);
+ if (exmap) delete exmap; // Remove() does not delete
+ }
+
+ if (reArchSubst) {
+ reArchSubst->Substitute(tUrl, substWith, kFALSE);
+ os->SetString(tUrl.Data());
+ }
+ else if (fAnchor) {
+ tUrl.Append("#");
+ tUrl.Append(fAnchor);
+ os->SetString(tUrl.Data());
+ }
}
- }
- }
+ if (reArchSubst) delete reArchSubst;
+ }
- return fGridResult;
+ return fGridResult;
}
//______________________________________________________________________________
@@ -114,9 +163,10 @@ const char *TAliEnFind::GetSearchId()
{
if (fSearchId.IsNull()) {
TString searchIdStr;
- searchIdStr.Form("BasePath=%s FileName=%s Anchor=%s TreeName=%s Regexp=%s",
- fBasePath.Data(), fFileName.Data(), fAnchor.Data(), fTreeName.Data(),
- fRegexpRaw.Data());
+ searchIdStr.Form("BasePath=%s FileName=%s Anchor=%s ArchSubst=%d "
+ "TreeName=%s Regexp=%s",
+ fBasePath.Data(), fFileName.Data(), fAnchor.Data(), fArchSubst,
+ fTreeName.Data(), fRegexpRaw.Data());
TMD5 *md5 = new TMD5();
md5->Update( (const UChar_t *)searchIdStr.Data(),
(UInt_t)searchIdStr.Length() );
@@ -130,9 +180,9 @@ const char *TAliEnFind::GetSearchId()
}
//______________________________________________________________________________
-TFileCollection *TAliEnFind::GetCollection()
+TFileCollection *TAliEnFind::GetCollection(Bool_t forceNewQuery)
{
- if (!fGridResult) GetGridResult();
+ GetGridResult(forceNewQuery);
if (!fGridResult) return NULL;
Int_t nEntries = fGridResult->GetEntries();
@@ -163,6 +213,91 @@ TFileCollection *TAliEnFind::GetCollection()
return fc;
}
+//______________________________________________________________________________
+void TAliEnFind::Print(Option_t* opt) const
+{
+ if (opt) {} // silence warning
+ Printf("BasePath=%s FileName=%s Anchor=%s ArchSubst=%d "
+ "TreeName=%s Regexp=%s (query %s a result)",
+ fBasePath.Data(), fFileName.Data(), fAnchor.Data(), fArchSubst,
+ fTreeName.Data(), fRegexpRaw.Data(), (fGridResult ? "has" : "has not"));
+}
+
+//______________________________________________________________________________
+void TAliEnFind::SetBasePath(const char *basePath)
+{
+ if (fBasePath.EqualTo(basePath)) return;
+ fBasePath = basePath;
+ InvalidateGridResult();
+ InvalidateSearchId();
+}
+
+//______________________________________________________________________________
+void TAliEnFind::SetFileName(const char *fileName)
+{
+ if (fFileName.EqualTo(fileName)) return;
+ fFileName = fileName;
+ InvalidateGridResult();
+ InvalidateSearchId();
+}
+
+//______________________________________________________________________________
+void TAliEnFind::SetAnchor(const char *anchor)
+{
+ if (fAnchor.EqualTo(anchor)) return;
+ fAnchor = anchor;
+ InvalidateGridResult();
+ InvalidateSearchId();
+}
+
+//______________________________________________________________________________
+void TAliEnFind::SetTreeName(const char *treeName)
+{
+ if (fTreeName.EqualTo(treeName)) return;
+ fTreeName = treeName;
+ InvalidateSearchId();
+}
+
+//______________________________________________________________________________
+void TAliEnFind::SetArchSubst(Bool_t archSubst)
+{
+ if (fArchSubst == archSubst) return;
+ fArchSubst = archSubst;
+ InvalidateGridResult();
+ InvalidateSearchId();
+}
+
+//______________________________________________________________________________
+void TAliEnFind::SetRegexp(const char *regexp)
+{
+ if (fRegexpRaw.EqualTo(regexp)) return;
+
+ fRegexpRaw = regexp;
+ if (fRegexp) delete fRegexp;
+ if (!fRegexpRaw.IsNull())
+ fRegexp = new TPMERegexp(regexp);
+ else
+ fRegexp = NULL;
+
+ InvalidateGridResult();
+ InvalidateSearchId();
+}
+
+//______________________________________________________________________________
+void TAliEnFind::InvalidateSearchId()
+{
+ if (!fSearchId.IsNull())
+ fSearchId = "";
+}
+
+//______________________________________________________________________________
+void TAliEnFind::InvalidateGridResult()
+{
+ if (fGridResult) {
+ delete fGridResult;
+ fGridResult = NULL;
+ }
+}
ClassImp(TDataSetManagerAliEn);
@@ -475,29 +610,29 @@ Bool_t TDataSetManagerAliEn::ParseOfficialDataUri(TString &uri, Bool_t sim,
}
//
- // Parse run list
+ // Parse pass: mandatory on Data, useless on Sim
//
- TPMERegexp reRun("(^|;)Run=([0-9,-]+)(;|$)");
- if (reRun.Match(uri) != 4) {
+ TPMERegexp rePass("(^|;)Pass=([a-zA-Z_0-9-]+)(;|$)");
+ if ((rePass.Match(uri) != 4) && (!sim)) {
Error("ParseOfficialDataUri",
- "Run or run range not specified (e.g., Run=139104-139107,139306)");
+ "Pass (e.g., Pass=cpass1_muon) is mandatory on real data");
return kFALSE;
}
- TString runListStr = reRun[2];
- runList = ExpandRunSpec(runListStr);
+ pass = rePass[2];
//
- // Parse pass: mandatory on Data, useless on Sim
+ // Parse run list
//
- TPMERegexp rePass("(^|;)Pass=([a-zA-Z_0-9-]+)(;|$)");
- if ((rePass.Match(uri) != 4) && (!sim)) {
+ TPMERegexp reRun("(^|;)Run=([0-9,-]+)(;|$)");
+ if (reRun.Match(uri) != 4) {
Error("ParseOfficialDataUri",
- "Pass (e.g., Pass=cpass1_muon) is mandatory on real data");
+ "Run or run range not specified (e.g., Run=139104-139107,139306)");
return kFALSE;
}
- pass = rePass[2];
+ TString runListStr = reRun[2];
+ runList = ExpandRunSpec(runListStr); // must be freed by caller
return kTRUE;
}
@@ -553,14 +688,15 @@ std::vector<Int_t> *TDataSetManagerAliEn::ExpandRunSpec(TString &runSpec) {
// Remove duplicates
{
- std::vector<Int_t>::iterator itr, prev;
- for (itr=runNums.begin(); itr!=runNums.end(); itr++) {
- if (itr == runNums.begin()) continue;
- prev = itr;
- prev--;
- if (*prev == *itr) {
- runNums.erase(itr);
- itr--;
+ std::vector<Int_t>::iterator itr = runNums.begin();
+ Int_t prevVal;
+ while (itr != runNums.end()) {
+ if ((itr == runNums.begin()) || (prevVal != *itr)) {
+ prevVal = *itr;
+ itr++;
+ }
+ else {
+ itr = runNums.erase(itr);
}
}
}
@@ -613,7 +749,6 @@ TFileCollection *TDataSetManagerAliEn::GetDataSet(const char *uri, const char *)
if (gDebug >= 1)
Info("GetDataSet", "Getting file collection from AliEn");
- //newFc = AliEnFind(*af, anchor, treeName, archSubst, kFALSE);
newFc = af->GetCollection();
if (!newFc) {
Error("GetDataSet", "Cannot get collection from AliEn");
diff --git a/proof/proof/src/TProof.cxx b/proof/proof/src/TProof.cxx
index e4631e49411..66bd485d90f 100644
--- a/proof/proof/src/TProof.cxx
+++ b/proof/proof/src/TProof.cxx
@@ -11000,7 +11000,7 @@ TFileCollection *TProof::GetStagingStatusDataSet(const char *dataset)
nameMess << Int_t(kStagingStatus);
nameMess << TString(dataset);
if (Broadcast(nameMess) < 0) {
- Error("StagingStatusDataSet", "sending request failed");
+ Error("GetStagingStatusDataSet", "sending request failed");
return NULL;
}
@@ -11008,7 +11008,7 @@ TFileCollection *TProof::GetStagingStatusDataSet(const char *dataset)
TFileCollection *fc = NULL;
if (fStatus < 0) {
- Error("StagingStatusDataSet", "problem processing the request");
+ Error("GetStagingStatusDataSet", "problem processing the request");
}
else if (fStatus == 0) {
TMessage *retMess = (TMessage *)fRecvMessages->First();
@@ -11016,10 +11016,10 @@ TFileCollection *TProof::GetStagingStatusDataSet(const char *dataset)
fc = (TFileCollection *)(
retMess->ReadObject(TFileCollection::Class()) );
if (!fc)
- Error("StagingStatusDataSet", "error reading list of files");
+ Error("GetStagingStatusDataSet", "error reading list of files");
}
else {
- Error("StagingStatusDataSet",
+ Error("GetStagingStatusDataSet",
"response message not found or wrong type (%p)", retMess);
}
}
--
GitLab