Fix microtar tar header size field on MinGW Windows#7467
Merged
DennisOSRM merged 1 commit intoProject-OSRM:masterfrom Apr 11, 2026
Merged
Fix microtar tar header size field on MinGW Windows#7467DennisOSRM merged 1 commit intoProject-OSRM:masterfrom
DennisOSRM merged 1 commit intoProject-OSRM:masterfrom
Conversation
On Windows (LLP64), `long` is 32-bit even on x86_64. The `_MSC_VER` guard in microtar correctly used `%llo` for MSVC but fell through to `%lo` for MinGW, causing: - sscanf with `%lo`: writes only 4 bytes into the 8-byte `h->size` field, leaving upper bytes uninitialized. Any subsequent size comparison fails, producing "Datatype size does not match file size" errors on osrm_fingerprint.meta. - sprintf with `%lo`: reads only the lower 32 bits, producing incorrect tar headers for files above 4 GB. Replace the platform-specific `_MSC_VER` ifdef with portable `uint64_t` and `<inttypes.h>` format macros (`SCNo64`/`PRIo64`), which are correct on all platforms and compilers.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes incorrect TAR header size field formatting/parsing on MinGW Windows (LLP64) by avoiding long-based %lo formatting and switching to uint64_t + <inttypes.h> format macros, ensuring correct behavior across compilers/platforms (notably for downstream Julia builds).
Changes:
- Add
<inttypes.h>include for portableSCNo64/PRIo64usage. - Parse the TAR size field into a
uint64_tviaSCNo64, then cast tomtar_size_t. - Format the TAR size field using
PRIo64with an explicituint64_tcast.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
DennisOSRM
approved these changes
Apr 11, 2026
Contributor
Author
|
Thanks @DennisOSRM for swiftly merging this. Would it be possible to get a point release? |
Collaborator
new release coming soon |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On Windows (LLP64),
longis 32-bit even on x86_64. The_MSC_VERguard in microtar correctly used%llofor MSVC but fell through to%lofor MinGW, causing the bug in the Julia binary of OSRM.Replace the platform-specific
_MSC_VERifdef with portableuint64_tand<inttypes.h>format macros (SCNo64/PRIo64), which are correct on all platforms and compilers.With this fix, we get the tests in the Julia wrapper passing. See OpenSourceRoutingMachine.jl and the build recipe, which contains the patch currently.
PS: It would be great if we could get a new point release we could refer to downstream once this is merged.