-
Notifications
You must be signed in to change notification settings - Fork 12
Support for files over 2GB on Windows #36
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
Support for files over 2GB on Windows #36
Conversation
## Problem File::Stat fails with RuntimeError when handling files >= 2GB on Windows. The stat operation crashes when attempting to get file statistics for large files, making it impossible to work with files over 2GB. ## Cause Windows `stat()` function uses 32-bit integers for file sizes by default, causing truncation for files >= 2GB (2^31 bytes). The regular `stat()` function and struct stat are limited to 32-bit file sizes on Windows, while Unix systems typically use 64-bit by default. ref: https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/stat-functions?view=msvc-170#time-type-and-file-length-type-variations-of-_stat ## Solution Use Windows 64-bit stat functions and structures: - Replace `stat()` with `_stat64()` for file statistics - Replace `fstat()` with _fstat64() for file descriptor statistics - Replace struct stat with struct `_stat64` for data structures - Define macros (STAT, LSTAT, FSTAT, STAT_STRUCT) to use 64-bit variants on Windows while keeping original functions on Unix This enables proper handling of files >= 2GB on Windows platforms while maintaining compatibility with other operating systems.
574a33c to
e406eb1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@otegami
Thank you for the suggestion. I agree with the policy of supporting large file sizes on Windows.
I'm a bit concerned about the STAT_STRUCT macro in terms of overall consistency. Instead of defining STAT_STRUCT, how about defining a structure like mrb_stat, so that we can write struct mrb_stat, and let mrb_stat branch into platform-specific structures? (Although I am a little worried that the name is too similar to mrb_state...)
test/file-stat.rb
Outdated
| target_size = 2**31 # 2GB | ||
| begin | ||
| File.open(large_file, 'wb') do |f| | ||
| (2**19).times { f << "\0" * 4096 } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use target_size here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix: e15ed5b Sure, I tried to use it with seek.
Improve code consistency by replacing the STAT_STRUCT macro with a more conventional mrb_stat struct name approach: - Windows: mrb_stat -> _stat64 (for 64-bit file support) - Unix/Linux: mrb_stat -> stat (standard struct)
|
fix: 34be575 Thank you for reviewing! I understand your concerns and have followed your suggestion to define a new struct to absorb the differences between platforms. Regarding the name, how about using |
|
LGTM. Since I think it will be easier to refer to if we align with the implementation in ruby/ruby, I believe it’s fine to keep the name as |
src/file-stat.c
Outdated
| # define STAT(p,s) _stat64(p,s) | ||
| # define FSTAT(fd,s) _fstat64(fd,s) | ||
| # define LSTAT(p,s) _stat64(p,s) | ||
| # define mrb_stat _stat64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, typedef is better than #define for type aliasing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix: 7a7009e Thank you so much. I got it.
cd96202 to
7a7009e
Compare
This PR updates `mruby-file-stat` submodule from f3e858f01 to 7d0e63a95 to add support for files over 2GB on Windows. - ref: ksss/mruby-file-stat#36 The previous stat implementation didn't support files larger than 2GB on Windows platforms. This update is necessary to allow the grndb command to properly check file stats for database files over 2GB on Windows.
Problem
File::Stat fails with RuntimeError when handling files >= 2GB on Windows. The stat operation crashes when
attempting to get file statistics for large files, making it impossible to work with files over 2GB.
Cause
Windows
stat()function uses 32-bit integers for file sizes by default, causing truncation for files >= 2GB (2^31 bytes). The regularstat()function and struct stat are limited to 32-bit file sizes on Windows, while Unix systems typically use 64-bit by default.ref: https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/stat-functions?view=msvc-170#time-type-and-file-length-type-variations-of-_stat
Solution
Use Windows 64-bit stat functions and structures:
stat()with_stat64()for file statisticsfstat()with _fstat64() for file descriptor statistics_stat64for data structuresThis enables proper handling of files >= 2GB on Windows platforms while maintaining compatibility with other operating systems.