Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[IO] Improve error message when no TFile::Open for remote files #11719

Merged
merged 2 commits into from
Nov 16, 2022

Conversation

eguiraud
Copy link
Member

@eguiraud eguiraud commented Nov 16, 2022

In order to mark the file as zombie when this happens, I had to change a goto zombie into calls to a dedicated helper lambda (done in a separate commit). Otherwise I would have had to move around a few lines just so that the compiler would let me jump from the sanity check to the zombie label (compilers don't allow jumps across variable initializations).

This fixes #10039.

Besides making control flow harder to follow, it makes it harder
to introduce other early returns as compilers will not allow goto
calls that cross variable initializations.
@eguiraud eguiraud self-assigned this Nov 16, 2022
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/soversion, ROOT-performance-centos8-multicore/cxx17, ROOT-ubuntu18.04/nortcxxmod, ROOT-ubuntu2004/python3, mac12/noimt, mac11/cxx14, windows10/cxx14
How to customize builds

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.

Looks good. Thanks. Consider adding the minor/unrelated improvement I suggested.

Comment on lines +324 to +325
R__LOCKGUARD(gROOTMutex);
gROOT->GetListOfClosedObjects()->Add(this);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
R__LOCKGUARD(gROOTMutex);
gROOT->GetListOfClosedObjects()->Add(this);
gROOT->GetListOfClosedObjects()->Add(this);

The list of Closed Objects has its won locked nowadays (yes, I don't know why it was not removed sooner).

Copy link
Member Author

Choose a reason for hiding this comment

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

this looks completely unrelated, it should probably be done in a separate PR for all occurrences of the pattern

@eguiraud eguiraud merged commit dfb707c into root-project:master Nov 16, 2022
@eguiraud eguiraud deleted the tfile-error-message branch November 16, 2022 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TFile's ctor's error message should point to TFile::Open when filename contains "://"
3 participants