refactor: Return CAutoFile from BlockManager::Open*File() #28483
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
DrahtBot
changed the title
sedited
left a comment
sedited
left a comment
There was a problem hiding this comment.
ACK fa33b6fba270e5867f208b242f50145e1ed3550e
src/streams.h
Outdated
There was a problem hiding this comment.
Nit: Would removing this in the second commit when the FILE * is changed to CAutoFile make more sense?
There was a problem hiding this comment.
Now that fclose is removed, copies are allowed, because they no longer lead to a double-free undefined behavior segfault (Calling fclose twice on the same pointer)
This was only explicitly used in the tests, where it can be replaced by wrapping the original raw file pointer into a CAutoFile on creation and then calling CAutoFile::fclose(). Also, it was used in LoadExternalBlockFile(), where it can also be replaced by the (implicit call to the) CAutoFile destructor after wrapping the original raw file pointer in a CAutoFile.
This refactor allows to forward some calls to the underlying CAutoFile, instead of re-implementing the logic in the buffered file.
This is a refactor.
maflcko
force-pushed
the
2309-rework-block-file-
branch
from
fa33b6f to
fa56c42
Compare
willcl-ark
left a comment
willcl-ark
left a comment
There was a problem hiding this comment.
tACK fa56c42
Clean refactor needed for future work.
5 participants
This is required for #28052, but makes sense on its own, because offloading logic to
CAutoFileinstead of re-implementing it allows to delete code and complexity.