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

Implement and test file seek, extract common "read_exactly" for reuse #368

Merged
merged 2 commits into from
Dec 3, 2023

Conversation

BenWiederhake
Copy link
Contributor

This PR:

  • Extracts the private method osmium::io::detail::PbfParser::read_exactly as a generic, easily reusable function
  • Implements osmium::util::file_seek for random access in files (currently unused, see below)
  • Tests the seek feature (which also uses read_exactly)

This works toward #367, which is where file_seek will be very useful.

If you think file_seek should instead live in experimental, I'd be happy to move the function there.

@BenWiederhake
Copy link
Contributor Author

Changes since last push:

  • Added #include <cassert> to read_write.hpp, since I use assert(). Found through iwyu, even though its output is incredibly noisy.

@BenWiederhake
Copy link
Contributor Author

Changes since last push:

  • Rebased on current master
  • Marked read_exactly as "inline" instead of "static"
  • Reproduce the same build environment to make sure there are no further such issues

Copy link
Member

@joto joto 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, except the two small things I wrote down.

I am getting some weird effects here that github actions seems not to be testing your newsest commit. Hopefully this sorts itself out when you add those changes.

include/osmium/util/file.hpp Outdated Show resolved Hide resolved
test/t/io/test_file_seek.cpp Outdated Show resolved Hide resolved
@BenWiederhake
Copy link
Contributor Author

Changes since last push:

  • Added the cast to __int64, that seems to be the type that's also used in other MSVC-specific implementations, like resize_file.
  • Removed the print from the test

@joto joto merged commit f79e8aa into osmcode:master Dec 3, 2023
35 checks passed
@BenWiederhake BenWiederhake deleted the dev-seek branch December 3, 2023 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants