Skip to content

Conversation

@silverweed
Copy link
Contributor

This Pull request:

adds a method to TMemFile that allows to dump the current contents of the memory blocks to file. Differently from Cp, it does not modify the contents in any way, so it's useful/meant to be used for debugging.

The doc comment clarifies that Cp should still be preferred for regular use cases.

@silverweed silverweed requested a review from jblomer December 3, 2025 12:38
@silverweed silverweed self-assigned this Dec 3, 2025
@silverweed silverweed requested a review from pcanal as a code owner December 3, 2025 12:38
Copy link
Contributor

@jblomer jblomer left a comment

Choose a reason for hiding this comment

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

Nice!

Perhaps better implemented as ROOT::Internal::DumpBin(TMemFile &) in order to not increase the public interface.

@silverweed silverweed changed the title [io] Add TMemFile::DumpBin for debugging purposes [io] Add ROOT::Internal::DumpBin(TMemFile&) for debugging purposes Dec 3, 2025
@github-actions
Copy link

github-actions bot commented Dec 3, 2025

Test Results

    21 files      21 suites   3d 13h 54m 47s ⏱️
 3 785 tests  3 737 ✅ 0 💤 48 ❌
77 529 runs  77 481 ✅ 0 💤 48 ❌

For more details on these failures, see this check.

Results for commit 8eb21db.

Comment on lines +321 to +323
/// FILE *out = fopen("memfile_dump.root", "wb");
/// ROOT::Internal::DumpBin(memFile, out);
/// fclose(out);
Copy link
Member

Choose a reason for hiding this comment

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

What is the advantage of requiring the 3 steps instead of folding them into the function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mostly the ability to pass stdout or similar, which would be more awkward to do with a string argument.
Also one may potentially want to dump multiple MemFiles to the same output, though that's probably rare.

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.

LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants