Skip to content

[roottest] fix leak when using TSystem::ConcatFileName #18726

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
May 15, 2025

Conversation

linev
Copy link
Member

@linev linev commented May 14, 2025

Return value must be released.

@linev linev requested a review from pcanal May 14, 2025 14:04
@linev linev self-assigned this May 14, 2025
@linev linev requested a review from dpiparo as a code owner May 14, 2025 14:04
@ferdymercury
Copy link
Collaborator

ferdymercury commented May 14, 2025

Thanks for noticing! I think there are two more leaks with the same function in RooWorkspace.cxx ? Since the returned value is "copied" to a string rather than "moved" ? fyi @guitargeek
Or it should use the helper function TSystem.cxx:AssignAndDelete

Copy link

github-actions bot commented May 14, 2025

Test Results

    18 files      18 suites   3d 10h 25m 57s ⏱️
 2 741 tests  2 741 ✅ 0 💤 0 ❌
48 184 runs  48 184 ✅ 0 💤 0 ❌

Results for commit 48dca1e.

♻️ This comment has been updated with latest results.

Copy link
Member

@pcanal pcanal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@linev linev force-pushed the roottest_memleak branch from a916a0a to 48dca1e Compare May 15, 2025 08:01
@linev linev merged commit b65a5d6 into root-project:master May 15, 2025
21 of 23 checks passed
@linev linev deleted the roottest_memleak branch May 15, 2025 10:29
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants