[RF] Fix memory leak of ConcatFileName return value in RooWorkspace #18730
Conversation
Fix a memory leak of the `TSystem::ConcatFileName()` return value in RooWorkspace, and also refactor the code to avoid repetition. Thanks a lot to @ferdymercury for noticing this!
guitargeek
force-pushed
the
workspace_memleak
branch
from
bc4454c to
b4b9226
Compare
Test Results 19 files 19 suites 3d 10h 49m 20s ⏱️ Results for commit b4b9226. ♻️ This comment has been updated with latest results. |
| // Check list of additional paths | ||
| for (std::string const &diter : dirList) { | ||
|
|
||
| char *cpath = gSystem->ConcatFileName(diter.c_str(), file.c_str()); |
There was a problem hiding this comment.
This ConcatFileName function is horrible! Any chance we could deprecate it and provide a new one that returns a std::string? (Looking at its implementation it doesn't even make sense that it returns a char * rather than a TString...)
There was a problem hiding this comment.
Of course it is horrible, haha! But can you maybe still approve the fix I have now, so I can merge and backport it? For the future, I agree we should absolutely get rid of this function.
There was a problem hiding this comment.
Yes, sorry, I wasn't implying that it should be done in this PR, just something to think of for the future :)
Fix a memory leak of the
TSystem::ConcatFileName()return value in RooWorkspace, and also refactor the code to avoid repetition.Thanks a lot to @ferdymercury for noticing this!
#18726 (comment)