Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Jun 29, 2023

This improves FileCommit: The patch removes the requirement from the call site to open or obtain a raw C-style FILE pointer. Now the call site can use any style of file IO.

There are many other smaller improvements, which are explained in each commit.

@DrahtBot
Copy link
Contributor

DrahtBot commented Jun 29, 2023

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #26654 (util: Show descriptive error messages when FileCommit fails by john-moffett)

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 DrahtBot changed the title refactor: Open file in FileCommit refactor: Open file in FileCommit Jun 29, 2023
@maflcko maflcko force-pushed the 2306-fs-stuff- branch 2 times, most recently from fa9350f to 624ba04 Compare June 29, 2023 18:24
@maflcko
Copy link
Member Author

maflcko commented Jun 30, 2023

Closing for now, until there is a higher need for it.

@maflcko maflcko closed this Jun 30, 2023
@maflcko maflcko deleted the 2306-fs-stuff- branch June 30, 2023 05:46
@maflcko maflcko restored the 2306-fs-stuff- branch July 20, 2023 06:49
@maflcko
Copy link
Member Author

maflcko commented Jul 20, 2023

Looks like this will make it safer to remove std::ftell when using XOR, so I'll reopen for now as draft.

@maflcko maflcko reopened this Jul 20, 2023
@maflcko maflcko marked this pull request as draft July 20, 2023 06:50
@DrahtBot
Copy link
Contributor

🐙 This pull request conflicts with the target branch and needs rebase.

MarcoFalke added 3 commits July 20, 2023 20:12
This is needed for a future commit.
This allows to remove platform-specific code and avoids it being
maintained in this codebase.

This is a refactor, apart from potentially using one more file
descriptor at runtime, due to the same file being opened already
earlier. However, the increase in file descriptors will be undone in the
next commit.
* This removes the requirement from the call site to use a raw C-style
  FILE pointer. Now the call site can use any style of file IO.
* This makes the edge-case impossible to call FileCommit with nullptr,
  which would eventually lead to a segfault.
* This removes the burden from the call site to first open the file.
* This avoids a fseek if the file was opened using FlatFileSeq::Open
  with a non-zero nPos.
* Where the file was already open at a call site, it removes the burden
  to keep it open for FileCommit. Call sites are now expected to close
  or flush the file before calling FileCommit.
* This removes the requirement from the call site to open the file with
  the right mode (write).

This should be a refactor, because the following corner cases never happen:
* FlatFileSeq::Open has a side-effect to create the file when it is
  missing. Thus, in WriteFilterToDisk, if the first filter overflew the
  first file, the node would abort. Previously it would create an empty
  first filter file.
@maflcko
Copy link
Member Author

maflcko commented Jul 20, 2023

Closing for now.

@maflcko maflcko closed this Jul 20, 2023
@maflcko maflcko deleted the 2306-fs-stuff- branch July 20, 2023 19:20
@bitcoin bitcoin locked and limited conversation to collaborators Jul 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants