Skip to content

Improving mmap_windows.go #162

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

Closed
database64128 opened this issue Jan 13, 2025 · 1 comment · Fixed by #164
Closed

Improving mmap_windows.go #162

database64128 opened this issue Jan 13, 2025 · 1 comment · Fixed by #164

Comments

@database64128
Copy link
Contributor

There are a few issues with the current mmap implementation for Windows.

  1. The file mapping handle can be closed right after calling MapViewOfFile. There's no need to keep it in a map.
  2. On 32-bit systems, with large files, MapViewOfFile may return a partial mapping. The VirtualQuery should be called on the returned address to determine the actual size of the mapping, and return an error if the mapping is incomplete. See https://github.com/golang/go/blob/master/src/cmd/go/internal/mmap/mmap_windows.go.
  3. The mapping is opened with read-only access. No need to call FlushViewOfFile at all.

I can open a PR to address these issues, if you are OK with it.

These changes would be essentially a rewrite of mmap_windows.go, so the special license can be removed as well.

@oschwald
Copy link
Owner

That all sounds great. I don't personally use Windows and would be thankful for any help improving its support.

database64128 added a commit to database64128/maxminddb-golang that referenced this issue Jan 20, 2025
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 a pull request may close this issue.

2 participants