Skip to content

Fix Windows handling of large files#7437

Merged
DennisOSRM merged 1 commit intoProject-OSRM:masterfrom
861:Fix-Windows-Build-Bug
Mar 31, 2026
Merged

Fix Windows handling of large files#7437
DennisOSRM merged 1 commit intoProject-OSRM:masterfrom
861:Fix-Windows-Build-Bug

Conversation

@861
Copy link
Copy Markdown
Contributor

@861 861 commented Mar 29, 2026

Issue

What issue is this PR targeting? If there is no issue that addresses the problem, please open a corresponding issue and link it here.

This PR is targeting OSRM windows build. issue #7308

This issue was caused by fseek function in microtar library. According to microsoft doc , on windows _fseeki64 is an better option.

@861
Copy link
Copy Markdown
Contributor Author

861 commented Mar 29, 2026

@DennisOSRM I tested the entire US dataset today using OSRM V6.0.0 with this patch, it is working now on Windows 11 and Windows server 2019.

Copy link
Copy Markdown
Collaborator

@DennisOSRM DennisOSRM left a comment

Choose a reason for hiding this comment

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

Good catch!

static int file_seek(mtar_t *tar, mtar_size_t offset) {
int res = fseek(tar->stream, offset, SEEK_SET);
int res;
#if defined(_WIN32)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No need to support 32 bit environment. We dropped support some time ago.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Marco _WIN32 is defined for both 32-bit and 64-bit. Do you want me to update it to _WIN64 for 64-bit only?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If I understand this right, then we don‘t need the if defined block and can replace it with this single line:

int res = _fseeki64(tar->stream, (__int64) offset, SEEK_SET);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

_fseeki64 is windows only. Not available in linux, so if the os is not windows, we still need to use fseek.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah! Thanks for the clarification.

So, yes, if there's a WINDOWS or WIN64 define to indicate Windows environment, then let's use that. Otherwise, your patch looks good.

@DennisOSRM DennisOSRM changed the title Fix windows build issue Fix Windows handling of large files Mar 31, 2026
@DennisOSRM DennisOSRM enabled auto-merge (squash) March 31, 2026 09:55
Copy link
Copy Markdown
Collaborator

@DennisOSRM DennisOSRM left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution to the code base

@DennisOSRM DennisOSRM merged commit 1b27ef8 into Project-OSRM:master Mar 31, 2026
23 checks passed
@861
Copy link
Copy Markdown
Contributor Author

861 commented Mar 31, 2026

Thank you for your contribution to the code base

No problem. I am very glad i can make contribution.

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.

2 participants